Move custom code to dedicated files and cleanup v1 vs v2 checks#331
Merged
pirate merged 3 commits intopydantic-extractfrom Apr 1, 2026
Merged
Move custom code to dedicated files and cleanup v1 vs v2 checks#331pirate merged 3 commits intopydantic-extractfrom
pirate merged 3 commits intopydantic-extractfrom
Conversation
1 task
There was a problem hiding this comment.
1 issue found across 9 files
Confidence score: 4/5
- This PR looks safe to merge; the only issue is a test skip reason string mismatch, which is low impact and mostly clarity/documentation.
- In
tests/test_models.py, the skip reason referencesTypeAliasTypebut the test is unrelated, which could confuse future maintenance or test intent. - Pay close attention to
tests/test_models.py- ensure the skip reason accurately reflects why the test is skipped on v1.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_models.py">
<violation number="1" location="tests/test_models.py:836">
P2: Incorrect skip reason: this test has nothing to do with `TypeAliasType`. The reason string was copy-pasted from `test_type_alias_type` above. If skipping on v1 is intentional, update the reason to explain the actual incompatibility (e.g., `cls` field handling).</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as Client Code
participant Session as Session / AsyncSession
participant Patch as Session Extract Patch
participant PydanticUtil as Pydantic Extract Utility
participant API as Stagehand API
participant Pydantic as Pydantic V2
Note over Session, Patch: NEW: Patch installed via sessions_helpers.py
User->>Session: extract(schema=MyModel, ...)
Session->>Patch: NEW: _sync_extract / _async_extract
alt Schema is Pydantic Model
Patch->>PydanticUtil: is_pydantic_model(schema)
PydanticUtil-->>Patch: True
Patch->>PydanticUtil: pydantic_model_to_json_schema(MyModel)
PydanticUtil->>Pydantic: model_json_schema()
Pydantic-->>Patch: JSON Schema dict
end
Patch->>API: POST /v1/sessions/{id}/extract (with JSON schema)
API-->>Patch: SessionExtractResponse (raw JSON result)
Note right of Patch: CHANGED: Extract logic moved from Session to Patch
alt Pydantic Model used & Response exists
Patch->>PydanticUtil: NEW: validate_extract_response(result, MyModel)
PydanticUtil->>PydanticUtil: NEW: _validation_schema(cache lookup)
Note over PydanticUtil: Applies strict_response_validation flag
PydanticUtil->>Pydantic: model_validate(result)
alt Validation Fails
PydanticUtil->>PydanticUtil: _convert_dict_keys_to_snake_case()
PydanticUtil->>Pydantic: model_validate(normalized_result)
end
PydanticUtil-->>Patch: MyModel Instance (or raw data if fail)
end
Patch-->>User: SessionExtractResponse (data.result = MyModel)
Note over PydanticUtil, Pydantic: CHANGED: Pydantic v2 is now strictly required
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| assert DISCRIMINATOR_CACHE.get(UnionType) is discriminator | ||
|
|
||
|
|
||
| @pytest.mark.skipif(PYDANTIC_V1, reason="TypeAliasType is not supported in Pydantic v1") |
There was a problem hiding this comment.
P2: Incorrect skip reason: this test has nothing to do with TypeAliasType. The reason string was copy-pasted from test_type_alias_type above. If skipping on v1 is intentional, update the reason to explain the actual incompatibility (e.g., cls field handling).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_models.py, line 836:
<comment>Incorrect skip reason: this test has nothing to do with `TypeAliasType`. The reason string was copy-pasted from `test_type_alias_type` above. If skipping on v1 is intentional, update the reason to explain the actual incompatibility (e.g., `cls` field handling).</comment>
<file context>
@@ -791,6 +833,7 @@ class B(BaseModel):
assert DISCRIMINATOR_CACHE.get(UnionType) is discriminator
+@pytest.mark.skipif(PYDANTIC_V1, reason="TypeAliasType is not supported in Pydantic v1")
def test_type_alias_type() -> None:
Alias = TypeAliasType("Alias", str) # pyright: ignore
</file context>
Suggested change
| @pytest.mark.skipif(PYDANTIC_V1, reason="TypeAliasType is not supported in Pydantic v1") | |
| @pytest.mark.skipif(PYDANTIC_V1, reason="field named 'cls' is not supported in Pydantic v1") |
pirate
added a commit
that referenced
this pull request
Apr 2, 2026
* Support pydantic models as schemas for extract * pop schema * bump pydantic dep and linting fixes * update tests to remove pydantic v1 tests * Move custom code to dedicated files and cleanup v1 vs v2 checks (#331) * move custom code to dedicated files and cleanup v1 vs v2 checks * hard-require pydantic v2 or greater * lint * lint fixes after merge * Centralize extract patching in sessions helpers * Bound extract validation schema cache --------- Co-authored-by: Nick Sweeting <github@sweeting.me> Co-authored-by: Nick Sweeting <git@sweeting.me>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary by cubic
Moved Pydantic extract logic into a dedicated module and auto-patched
Session/AsyncSessionto use it. Hard-requiredpydanticv2 at import time and added tests for extract behavior.Refactors
src/stagehand/_session_extract.pyand auto-installed the extract patch insessions_helpers._pydantic_extract.pywith cached model config andstrict_response_validation.pydanticv2 in_compat.pywith a clear ImportError.session.pyby delegating extract logic and removing v1/v2 conditionals; added focused tests for session extract.Migration
stagehandnow requirespydantic>=2,<3. Install or upgrade to v2.pydantic-v2extra frompyproject.toml; use standardpydanticv2 installation.Written for commit 297fe85. Summary will update on new commits. Review in cubic