ref(seer): Filter private fields from explorer chat API response#113125
Merged
ref(seer): Filter private fields from explorer chat API response#113125
Conversation
The explorer chat GET endpoint was forwarding the full SeerRunState from Seer to users, exposing internal fields like usage/cost data, metadata, and coding agent state. Additionally, all models used extra="allow" which let any undeclared fields from Seer leak through. Change extra to "ignore" so undeclared fields are dropped at parse time, and mark internal fields (usage, metadata, coding_agents) with Field(exclude=True) so they are omitted from .dict() but still accessible as attributes for internal callers like autofix. Also declares fields the frontend consumes that were previously only passing through as extras: owner_user_id, todos, tool_links, tool_results, thinking_content, and ToolCall.id. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-actions
Bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 28e56e6. Configure here.
| diff: str = "" | ||
|
|
||
| class Config: | ||
| extra = "allow" |
Contributor
There was a problem hiding this comment.
FilePatch still allows extra fields to leak through
Low Severity
FilePatch retains extra = "allow" while every other model in the chat response serialization chain (ExplorerFilePatch, MemoryBlock, SeerRunState, etc.) was changed to extra = "ignore". Since FilePatch is nested inside ExplorerFilePatch which appears in MemoryBlock.file_patches and MemoryBlock.merged_file_patches, any undeclared fields Seer attaches to file patch objects can still leak through state.dict() to the API response.
Reviewed by Cursor Bugbot for commit 28e56e6. Configure here.
Member
Author
There was a problem hiding this comment.
This was intentional since there were some nested fields that were a bit painful to add types for right now. Seems fine to keep as-is for the short term, can follow up.
aliu39
approved these changes
Apr 16, 2026
chromy
added a commit
that referenced
this pull request
Apr 16, 2026
…nse (#113125)" This reverts commit a2b4584. Adding `Field(exclude=True)` to `SeerRunState.metadata` (and `coding_agents`, `usage`) causes `trigger_push_changes` to raise `SeerPermissionError` — the `open_pr` path calls `client.get_run(run_id)` which round-trips the state through `.dict()`, dropping `metadata` and failing the `state.metadata.get("group_id") != group.id` check in autofix_agent.py, which the endpoint surfaces as 404. Breaks `test_open_pr`, `test_open_pr_with_repo_name`, and `test_open_pr_without_repo_name` in `GroupAutofixEndpointExplorerRoutingTest`. Agent transcript: https://claudescope.sentry.dev/share/jrltmqqO9R5-5IFIdaBmPVdw_GfoseCtIvlM1JXrb90
1 task
Contributor
|
PR reverted: 0fa9f32 |
joshuarli
added a commit
that referenced
this pull request
Apr 16, 2026
Pydantic BaseModel (and similar metaclass-driven patterns) generates __init__, dict(), etc. at class definition time. When a test only instantiates models and calls inherited methods, no lines from the source file appear in coverage — so the coverage DB never links the test to the changed file, and the test gets skipped. Add _find_tests_by_imports(): for each changed non-test .py source file, grep test directories for "from <module> import" / "import <module>" and union those results into the coverage-based selection. Reproducer: #113125 changed src/sentry/seer/explorer/client_models.py (Pydantic models only), and test_group_ai_autofix.py was skipped even though it imports SeerRunState from that file.
rbro112
added a commit
that referenced
this pull request
Apr 17, 2026
A failing test slipped past selective testing earlier (thread). Upon investigation, this was because there was a change to a model-only file (`src/sentry/seer/explorer/client_models.py`) [PR](#113125). In the test file that shouldn't have been skipped (`tests/sentry/seer/endpoints/test_group_ai_autofix.py`), one model from that `client_models.py` file is used (`SeerRunState`), but the usage is only an instantiation and a `.dict()` call, both pydantic BaseModel methods. `test_group_ai_autofix.py` doesn't show up in coverage for `client_models.py` , due to no lines of code from `client_models.py` actually being executed (just base model lines executed). This addresses this whole by unioning coverage detected test files and any test files that have a direct import on a changed file. This will miss transitive dependencies, but this should be at least one additional line of defense against holes. --------- Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
joshuarli
added a commit
that referenced
this pull request
Apr 22, 2026
Pydantic BaseModel (and similar metaclass-driven patterns) generates __init__, dict(), etc. at class definition time. When a test only instantiates models and calls inherited methods, no lines from the source file appear in coverage — so the coverage DB never links the test to the changed file, and the test gets skipped. Add _find_tests_by_imports(): for each changed non-test .py source file, grep test directories for "from <module> import" / "import <module>" and union those results into the coverage-based selection. Reproducer: #113125 changed src/sentry/seer/explorer/client_models.py (Pydantic models only), and test_group_ai_autofix.py was skipped even though it imports SeerRunState from that file.
joshuarli
added a commit
that referenced
this pull request
Apr 22, 2026
Pydantic BaseModel (and similar metaclass-driven patterns) generates __init__, dict(), etc. at class definition time. When a test only instantiates models and calls inherited methods, no lines from the source file appear in coverage — so the coverage DB never links the test to the changed file, and the test gets skipped. Add _find_tests_by_imports(): for each changed non-test .py source file, grep test directories for "from <module> import" / "import <module>" and union those results into the coverage-based selection. Reproducer: #113125 changed src/sentry/seer/explorer/client_models.py (Pydantic models only), and test_group_ai_autofix.py was skipped even though it imports SeerRunState from that file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The explorer chat GET endpoint was forwarding the full
SeerRunStatefrom Seer to users viastate.dict(), exposing internal fields and metadata. All models also usedextra = "allow"which let any undeclared fields from Seer leak through to the response.Two changes:
extra = "ignore"on models used in the chat response, so undeclared fields from Seer are dropped at parse time rather than leaking through.dict()Field(exclude=True)on some fields so they're omitted from.dict()but still accessible as attributes for internal callers (autofix, night shift, etc.)Also declares fields the frontend consumes that were previously only passing through as Pydantic extras:
owner_user_id,todos,tool_links,tool_results,thinking_content, andToolCall.id.Related: https://github.com/getsentry/seer/pull/5803 — this change on the Sentry side makes the Seer-side stripping of fields unnecessary for the chat endpoint.