You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2026/09/09 18:25:05 UTC

[PR] fix(mcp): expose resolved query time boundaries [superset]

aminghadersohi opened a new pull request, #44105:
URL: https://github.com/apache/superset/pull/44105

   ### SUMMARY
   
   The query engine resolves a relative time range to concrete bounds, but MCP discarded them. Callers could not tell which range produced the numbers: a model can narrate one range while the query used another.
   
   Verified on `master`: `from_dttm`/`to_dttm` appear nowhere in `superset/mcp_service/`, while `superset/common/query_context_processor.py` computes the result payload containing those resolved bounds.
   
   Both `query_dataset` and `get_table` return `from_dttm` (inclusive) and `to_dttm` (exclusive), including on cached and empty results. The response schemas and tool documentation explain how to report these ISO 8601 values. Missing primary bounds remain null.
   
   **What this does not do:** no parser change; no year-offset bug was found. Callers can identify the engine's primary logical range, not every additional SQL filter or timezone-adjusted boundary. These fields are not observed row minima/maxima, and naive timestamps are not asserted to be UTC. Additional column filters, virtual dataset SQL, and semantic-view one-sided ranges rewritten as comparison filters can still constrain results.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Not applicable (MCP response metadata only).
   
   Before: the request's relative expression was available, but the resolved primary bounds were omitted.
   
   After: for example, `previous calendar month` evaluated on July 17, 2026 returns `from_dttm: "2026-06-01T00:00:00"` and `to_dttm: "2026-07-01T00:00:00"` (exclusive).
   
   ### TESTING INSTRUCTIONS
   
   - Run `pytest -q tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py`.
   - With MCP configured, query a temporal dataset using `query_dataset` and a relative `time_range`; compare the returned bounds with the engine result and generated primary time predicate.
   - Repeat using `get_table` for a built-in dataset and an external semantic view.
   - Repeat with a cache hit and with a query returning zero rows. Both fields should remain present and reflect the engine payload.
   - Check an unbounded query: missing primary bounds should serialize as null. Verify that descriptions distinguish an inclusive start from an exclusive end.
   
   Validation: all changed-file `uvx pre-commit run --files ...` hooks passed, including mypy and pylint. Targeted unit tests are in progress. A live-server check was unavailable because no service was listening on localhost:8088.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   Additive response fields only; no database migration or date-parser changes.
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #44105:
URL: https://github.com/apache/superset/pull/44105#discussion_r3975333947


##########
tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py:
##########
@@ -1406,3 +1408,75 @@ def capture_create(**kwargs):
     assert since is not None
     assert until is not None
     assert since < until
+
+
+@pytest.mark.parametrize(
+    ("expression", "expected_start", "expected_end"),
+    [
+        ("Last month", "2026-06-17T00:00:00", "2026-07-17T00:00:00"),
+        ("Last year", "2025-07-17T00:00:00", "2026-07-17T00:00:00"),
+        ("previous calendar month", "2026-06-01T00:00:00", "2026-07-01T00:00:00"),
+        ("Current year", "2026-01-01T00:00:00", "2027-01-01T00:00:00"),
+        ("2025-06-01 : 2025-07-01", "2025-06-01T00:00:00", "2025-07-01T00:00:00"),
+        ("No filter", None, None),
+    ],
+)
+@pytest.mark.parametrize("use_filter", [False, True])
+@pytest.mark.parametrize("result_kind", ["fresh", "cached", "empty"])
+@pytest.mark.asyncio
+async def test_query_dataset_returns_engine_time_bounds(
+    mcp_server: FastMCP,
+    expression: str,
+    expected_start: str | None,
+    expected_end: str | None,
+    use_filter: bool,
+    result_kind: str,
+) -> None:
+    """Resolve MCP inputs with the real factory and serialize execution bounds."""
+    from flask import current_app
+    from freezegun import freeze_time
+
+    from superset.common.chart_data import ChartDataResultType
+    from superset.common.query_object_factory import QueryObjectFactory
+
+    dataset = _make_dataset(main_dttm_col="order_date")
+
+    def execute(
+        datasource_id: int, datasource_type: str, query_dict: dict[str, Any], **_: Any
+    ) -> dict[str, Any]:
+        """Use production date resolution in place of database execution."""
+        factory = QueryObjectFactory(current_app.config, MagicMock())
+        with freeze_time("2026-07-17 12:34:56"):
+            query = factory.create(
+                parent_result_type=ChartDataResultType.FULL,
+                **query_dict,
+            )
+        payload = _mock_command_result()
+        result = payload["queries"][0]
+        result.update(from_dttm=query.from_dttm, to_dttm=query.to_dttm)
+        result["is_cached"] = result_kind == "cached"

Review Comment:
   Addressed with `test_query_dataset_cached_bounds_across_rollover`, covering both populated and empty results. It uses the real `QueryObjectFactory`, `QueryContextProcessor.get_df_payload`, and `QueryCacheManager` with a SimpleCache backend, so the first request serializes the cache entry and the second reconstructs it. The clock crosses midnight within the cache TTL; assertions verify identical cache keys, only one source execution, preserved cached rows, a real cache hit, and the new request's resolved bounds in the MCP JSON response.
   
   One important distinction from the suggested fix: the cache entry does not store `from_dttm`/`to_dttm`. Production obtains them from the current QueryObject when assembling the result, rather than restoring the original execution's bounds. The regression test explicitly verifies that behavior instead of seeding artificial cached bounds.
   



##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -270,6 +272,8 @@ def _build_response(
     )
 
     return GetTableResponse(
+        from_dttm=query_result.get("from_dttm"),
+        to_dttm=query_result.get("to_dttm"),

Review Comment:
   Checked this against the MCP entry point. The shared query builder supports that rewrite, but `GetTableRequest` validation rejects open-ended strings such as `"2025-01-01 : "` and `" : 2025-02-01"` before they reach execution. `validate_time_range` strips the input, which removes the space needed for the `" : "` separator, and the remaining string fails bounded-range validation.
   
   Added `test_get_table_rejects_open_ended_range_before_execution` for both directions and both dataset/view selectors. It calls the MCP tool and asserts the validation error and that `execute_tabular_query` is never called. Updated the documentation to distinguish this from the shared builder: callers can use explicit comparison filters for one-sided constraints, but those are not engine primary bounds. No synthetic bounds or query behavior changes are needed for this PR.
   



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44105:
URL: https://github.com/apache/superset/pull/44105#issuecomment-5616033809

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers an incremental AI Review.
   - `/review full` - Manually triggers a full AI Review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44105:
URL: https://github.com/apache/superset/pull/44105#issuecomment-5612518469

   <!-- Bito Reply -->
   The flagged issue is correct: the test case for cached results only verifies the `is_cached` flag and does not validate that the `from_dttm` and `to_dttm` fields are correctly reconstructed or serialized from the cache. To resolve this, the test should be updated to mock a cache entry containing specific time bounds and assert that these values are correctly returned in the `QueryDatasetResponse` when `is_cached` is true. I have not implemented the fix as the test file is not present in the provided PR context. Please provide the test file content if you would like me to implement the fix.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #44105:
URL: https://github.com/apache/superset/pull/44105#discussion_r3975258898


##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -270,6 +272,8 @@ def _build_response(
     )
 
     return GetTableResponse(
+        from_dttm=query_result.get("from_dttm"),
+        to_dttm=query_result.get("to_dttm"),

Review Comment:
   **Suggestion:** For external semantic views with one-sided time ranges, execution rewrites the range into comparison filters, so these fields are null despite a resolved primary bound. [api mismatch]
   
   **Assessment:** ๐ŸŸ  `Major` ยท ๐Ÿ” `Occurrence: Sometimes`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6935ec13dd7b446d961413ca003d9523&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6935ec13dd7b446d961413ca003d9523&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent ๐Ÿค– </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/mcp_service/semantic_layer/tool/get_table.py
   **Line:** 275:276
   **Comment:**
   	*Api Mismatch: For external semantic views with one-sided time ranges, execution rewrites the range into comparison filters, so these fields are null despite a resolved primary bound.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44105&comment_hash=df5d6cfce2799466eaab3d7dff3ea3575b76d365cfdc2a484b277426d94be454&reaction=like'>๐Ÿ‘</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44105&comment_hash=df5d6cfce2799466eaab3d7dff3ea3575b76d365cfdc2a484b277426d94be454&reaction=dislike'>๐Ÿ‘Ž</a>



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44105:
URL: https://github.com/apache/superset/pull/44105#issuecomment-5612565860

   @gabotorresruiz Thanks for the careful verification. Updated both `from_dttm` and `to_dttm` descriptions in `QueryDatasetResponse` and `GetTableResponse` in 16ecbfcac0 with: "On cache hits, these are current-request bounds; cached rows can reflect an earlier relative range. Check performance.cache_status."
   
   This explicitly covers the rollover case you verified: `QueryObject.cache_key` retains the relative `time_range` and removes `from_dttm`/`to_dttm`, so the resolved bounds do not establish the range of the cached rows. Description-only change; no behavior changes.
   
   Validation: `uvx pre-commit run --files <all branch-changed files>` passed, including mypy and pylint. Targeted unit tests were blocked during collection by local test-environment initialization. Before this push, CI had no failed checks, with 15 still pending/running. Left the PR ready for review (not draft).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #44105:
URL: https://github.com/apache/superset/pull/44105#discussion_r3975338845


##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -270,6 +272,8 @@ def _build_response(
     )
 
     return GetTableResponse(
+        from_dttm=query_result.get("from_dttm"),
+        to_dttm=query_result.get("to_dttm"),

Review Comment:
   โœ… **Customized review instruction saved!**
   
   **Instruction:**
   > Do not require synthetic primary time bounds for open-ended range strings in GetTable; validation rejects these inputs before execution, and callers should use explicit comparison filters for one-sided constraints.
   
   **Applied to:**
     - `superset/mcp_service/semantic_layer/tool/get_table.py`
   
   ---
   ๐Ÿ’ก *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44105:
URL: https://github.com/apache/superset/pull/44105#issuecomment-5612784147

   Addressed the review comments in d4b228a7e0. The cache-serialization suggestion is covered by a real cache write/read regression test across a frozen-clock rollover, for both populated and empty results; details are in the review-thread reply. The external one-sided-range report does not reproduce through MCP validation, so I added entry-point regression tests and clarified the documentation rather than changing query behavior.
   
   @gabotorresruiz Both bound descriptions retain the explicit rollover caveat. They point to `cache_status.cache_hit`: the new empty-result cache test confirms that `performance.cache_status` is `no_data` even on a cache hit.
   
   Validation: **134 targeted tool tests passed** with the local `superset-core/src` source path configured. `uvx pre-commit run --files <all branch-changed files>` passed, including mypy and pylint. CI had no failed checks before this push; remaining jobs were running. The PR remains ready for review, not draft.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44105:
URL: https://github.com/apache/superset/pull/44105#issuecomment-5606740460

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44105:
URL: https://github.com/apache/superset/pull/44105#issuecomment-5607005084

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44105?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 66.20%. Comparing base ([`70346a5`](https://app.codecov.io/gh/apache/superset/commit/70346a51b6999a6624406a1c801f0fc197947505?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`63b266a`](https://app.codecov.io/gh/apache/superset/commit/63b266ac2fce2cb77ad6d1ce995540609c9f0057?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 5 commits behind head on master.
   > :exclamation:  There is a different number of reports uploaded between BASE (70346a5) and HEAD (63b266a). Click for more details.
   > 
   > <details><summary>HEAD has 4 uploads less than BASE</summary>
   >
   >| Flag | BASE (70346a5) | HEAD (63b266a) |
   >|------|------|------|
   >|python|7|3|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #44105       +/-   ##
   ===========================================
   - Coverage   80.09%   66.20%   -13.90%     
   ===========================================
     Files        2922     2922               
     Lines      171996   172029       +33     
     Branches    39911    39922       +11     
   ===========================================
   - Hits       137765   113896    -23869     
   - Misses      31653    55755    +24102     
   + Partials     2578     2378      -200     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/44105/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage ฮ” | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/44105/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.39% <100.00%> (-0.01%)` | :arrow_down: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/44105/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/44105/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/44105/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.29% <100.00%> (-0.01%)` | :arrow_down: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/44105/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.81% <100.00%> (-27.69%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/44105/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.69% <100.00%> (-0.01%)` | :arrow_down: |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/44105/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44105?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #44105:
URL: https://github.com/apache/superset/pull/44105#discussion_r3975256547


##########
tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py:
##########
@@ -1406,3 +1408,75 @@ def capture_create(**kwargs):
     assert since is not None
     assert until is not None
     assert since < until
+
+
+@pytest.mark.parametrize(
+    ("expression", "expected_start", "expected_end"),
+    [
+        ("Last month", "2026-06-17T00:00:00", "2026-07-17T00:00:00"),
+        ("Last year", "2025-07-17T00:00:00", "2026-07-17T00:00:00"),
+        ("previous calendar month", "2026-06-01T00:00:00", "2026-07-01T00:00:00"),
+        ("Current year", "2026-01-01T00:00:00", "2027-01-01T00:00:00"),
+        ("2025-06-01 : 2025-07-01", "2025-06-01T00:00:00", "2025-07-01T00:00:00"),
+        ("No filter", None, None),
+    ],
+)
+@pytest.mark.parametrize("use_filter", [False, True])
+@pytest.mark.parametrize("result_kind", ["fresh", "cached", "empty"])
+@pytest.mark.asyncio
+async def test_query_dataset_returns_engine_time_bounds(
+    mcp_server: FastMCP,
+    expression: str,
+    expected_start: str | None,
+    expected_end: str | None,
+    use_filter: bool,
+    result_kind: str,
+) -> None:
+    """Resolve MCP inputs with the real factory and serialize execution bounds."""
+    from flask import current_app
+    from freezegun import freeze_time
+
+    from superset.common.chart_data import ChartDataResultType
+    from superset.common.query_object_factory import QueryObjectFactory
+
+    dataset = _make_dataset(main_dttm_col="order_date")
+
+    def execute(
+        datasource_id: int, datasource_type: str, query_dict: dict[str, Any], **_: Any
+    ) -> dict[str, Any]:
+        """Use production date resolution in place of database execution."""
+        factory = QueryObjectFactory(current_app.config, MagicMock())
+        with freeze_time("2026-07-17 12:34:56"):
+            query = factory.create(
+                parent_result_type=ChartDataResultType.FULL,
+                **query_dict,
+            )
+        payload = _mock_command_result()
+        result = payload["queries"][0]
+        result.update(from_dttm=query.from_dttm, to_dttm=query.to_dttm)
+        result["is_cached"] = result_kind == "cached"

Review Comment:
   **Suggestion:** The cached case only flips `is_cached`; it never exercises cache serialization or reconstruction, so missing or altered cached time bounds can pass this test. [incomplete implementation]
   
   **Assessment:** ๐ŸŸ  `Major` ยท ๐Ÿ” `Occurrence: Sometimes`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e89814cf867843e0b6786cad93ca7e0b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e89814cf867843e0b6786cad93ca7e0b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent ๐Ÿค– </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py
   **Line:** 1456:1457
   **Comment:**
   	*Incomplete Implementation: The cached case only flips `is_cached`; it never exercises cache serialization or reconstruction, so missing or altered cached time bounds can pass this test.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44105&comment_hash=0a9c9204044742309fee4b785b589cf5517472daaa45709310dce95a005db4a7&reaction=like'>๐Ÿ‘</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44105&comment_hash=0a9c9204044742309fee4b785b589cf5517472daaa45709310dce95a005db4a7&reaction=dislike'>๐Ÿ‘Ž</a>



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #44105:
URL: https://github.com/apache/superset/pull/44105


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44105:
URL: https://github.com/apache/superset/pull/44105#issuecomment-5612484934

   Thanks @gabotorresruiz โ€” addressed in d2f02a560b. Both response schemas now include the cache-hit caveat in the model-facing `to_dttm` description, and the query-date documentation carries the same warning. I used `cache_status.cache_hit` rather than `performance.cache_status` because the latter is `no_data` on empty-result responses, including cached empty results. No behavior or parser changes. Verified the generated JSON schemas, all changed-file pre-commit hooks passed (including mypy and pylint), and the two targeted tool test files passed all 128 tests.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(mcp): expose resolved query time boundaries [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44105:
URL: https://github.com/apache/superset/pull/44105#issuecomment-5606763439

   ### <span aria-hidden="true">โœ…</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">๐Ÿ”จ</span> Latest commit | 63b266ac2fce2cb77ad6d1ce995540609c9f0057 |
   |<span aria-hidden="true">๐Ÿ”</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6aa1a4822fcd7b0008d45d93 |
   |<span aria-hidden="true">๐Ÿ˜Ž</span> Deploy Preview | [https://deploy-preview-44105--superset-docs-preview.netlify.app](https://deploy-preview-44105--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">๐Ÿ“ฑ</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQ0MTA1LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.jQEfG6ooxzNS57KhiwaC8Pl-m8GwyIGtMNlE8XyLF6c)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">๐Ÿค–</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DYtBDoJADAD_0jMbF0RAf-Azyraym7C0KRsTY_y7Pc0cZr6gJlUbPAA6WA2PlN2xlmPLSGyn5HKpSYOxijXHKfubKbRSOXi_sY_EusvnSb5OiD2OyzC8Es1rjHGh8Ub3K_z-) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-44105--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org