aminghadersohi commented on PR #45484: URL: https://github.com/apache/superset/pull/45484#issuecomment-6331610754
@rebenitez1802 I addressed asks 1–3 in 839a6e8a77490bd04a5560dab0eeb45f85f3c3a7: 1. The cycle regression uses a synchronous cause-read cap and exact read-count assertion, with no thread. Regression test: test_classify_as_database_error_stops_on_cyclic_cause_chain. Removing the seen condition in a scratch copy produces an AssertionError, 1 failed in 1.51s (exit 1, not timeout). 2. build_query_dicts_from_form_data coerces a deep copy. Regression test: test_build_query_dicts_from_form_data_passes_enum_values_as_strings asserts the entire caller dictionary is unchanged, including nested enum identity, while the output grain is a plain string. Restoring in-place coercion in a scratch copy produces 1 failed in 1.48s. In 0f77e26adcf2758fce693b4f7df7fe4cc27111ad, test_table_inherited_offset_survives_filter_preparation and test_table_inherited_custom_offset_preserves_other_comparisons retain their query-output checks and assert the same caller-preservation contract across 20 comparison cases. 3. The log identifies dataset versus annotation classification through candidate engine specs and labels the main dataset only as context. Regression test: test_classification_debug_log_identifies_candidate_engine_source covers both sources; restoring the old wording produces 2 failed in 1.76s. The docs describe multiple candidate engines, unavailable annotation charts, and the specific-pattern-only fallback. 4. I left the optional heuristic simplification out: a specific-pattern-only rule changes the known-single-engine generic fallback. A scratch comparison returns annotation under the existing rule and non-connection under that proposed rule, so I cannot claim equivalence. Siblings checked: - tests/unit_tests/mcp_service/chart/test_review_query_and_preview_regressions.py:68 — fixed in 0f77e26adcf2758fce693b4f7df7fe4cc27111ad - superset/mcp_service/chart/chart_helpers.py:1192 — fixed in 839a6e8a77490bd04a5560dab0eeb45f85f3c3a7 - tests/unit_tests/mcp_service/chart/test_compile.py:1439 — fixed in 839a6e8a77490bd04a5560dab0eeb45f85f3c3a7 - superset/mcp_service/chart/compile.py:551 — fixed in 839a6e8a77490bd04a5560dab0eeb45f85f3c3a7 - superset/mcp_service/chart/chart_helpers.py:1194 — not affected: extra_form_data already uses a deep copy. - superset/mcp_service/chart/chart_helpers.py:1326 — not affected: the QueryContext wrapper delegates query construction to the copying builder. - superset/mcp_service/chart/compile.py:683 — not affected: native-reference validation already passes a deep copy. - superset/mcp_service/chart/tool/get_chart_data.py:869 — not affected: saved-chart fallback delegates to the same copying builder. - superset/mcp_service/chart/tool/generate_chart.py — not affected: saved creation and previews share the compile classifier. - superset/mcp_service/chart/tool/update_chart.py — not affected: table updates use shared compilation; semantic-view updates bypass table error classification. Validation: `pytest tests/unit_tests/mcp_service -n 4 -q -m 'not manual and not llm_compliance'` in the project virtual environment: 10944 passed, 12 skipped (exit 0). Changed-branch-files pre-commit passed, including mypy, ruff, and pylint. Real-provider tests were excluded; no provider API was called. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
