feat(python): read a model's declaration and check rule A (#1386) by yujiyokoo · Pull Request #1433 · cipherstash/stack

yujiyokoo · 2026-10-28T17:26:37Z

Summary

A Python developer marks a class as a model with @stackencrypt.model("users"). The decorator now reads the class's fields when the class is defined. A field without exactly one stash marker raises DeclarationError at the class statement, so the developer sees the mistake where they wrote it.

A stash marker says how a field is stored. TextEq encrypts it, Passthrough stores it as it is, and Ignore leaves it out.

Changes

_declaration.py: the model decorator and the field reader.
_errors.py: StashError and DeclarationError.
Tests for dataclasses, attrs, Pydantic, NamedTuple, and SQLAlchemy classes. Each test runs twice, with and without from __future__ import annotations.
pyproject.toml: attrs, Pydantic, and SQLAlchemy join the test extra only. The package itself imports none of them.

Verification

mise run test: 113 passed and 2 skipped, on Python 3.14.
The same tests on Python 3.9, in a separate environment: 113 passed and 2 skipped.
mypy --strict passes. pyright reports 0 errors.
pnpm run test:scripts: 83 test files pass.

Review notes

DECLARATION_MARKER_NOT_OUTERMOST is a new error code, for a marker in the wrong place, such as Annotated[Mapped[str], TextEq]. The spec's DECLARATION_NOT_OUTERMOST stays for the decorator order. The constructor check in python: declaration meaning — type table, hints, engine check, constructor #1387 finds that.
A relationship to a class defined later in the file is refused, because its hint does not resolve yet. The spec accepts this for v1.
On Python 3.14 without the future import, Python can fail to read the annotations itself. The error then names the class, not the field.

🤖 Generated with Claude Code

changeset-bot · 2026-10-28T17:26:43Z

⚠️ No Changeset found

Latest commit: bac90dd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

coderabbitai · 2026-10-28T17:26:47Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI; Enforced overrides: Organization
Review profile: CHILL
Plan: Advanced
Run ID: 2e8b50a8-285f-4387-9617-f2636e68263b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

Trigger review

`@stackencrypt.model("users")` reads a class's fields when the class is defined. A field without exactly one stash marker raises `DeclarationError` at the class statement. It works with dataclasses, attrs, Pydantic, `NamedTuple`, and SQLAlchemy classes. `DECLARATION_MARKER_NOT_OUTERMOST` is a new error code, for a marker in the wrong place. The spec's `DECLARATION_NOT_OUTERMOST` moves to #1387. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Codes such as `declaration_no_marker` and the reason `rule_a` follow the amendment on #1341. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cipherstash-agent

Review attestation

Reviewed bac90dd · posted from Claude Code · model claude-sonnet-5-5 · effort medium
Sources: code-review · claude-sonnet-5-5 (cross-checked inline by the posting session)
Skills: code-review, review-cross-check, review-post

Findings (4): 1 major, 3 minor — 4 inline, 0 below.

Two candidate findings were dropped: the pydantic __pydantic_fields__ fallback (refuted — on pydantic 2.9.2 get_type_hints returns only the declared fields) and the unbounded test extra (documented design decision in the PR).

Generated by Claude Code

cipherstash-agent · 2026-11-10T00:42:44Z

+    origin = get_origin(hint)
+    return (
+        isinstance(origin, type)
+        and origin.__module__ not in _VALUE_TYPE_MODULES


major _is_wrapper treats any one-argument generic outside _VALUE_TYPE_MODULES as a wrapper, so on a plain or SQLAlchemy class x: Annotated[re.Pattern[str], Passthrough] (or queue.Queue[int], Box[int]) is refused with declaration_marker_not_outermost and told to move a correctly placed marker. Restrict wrappers to a known set (e.g. Mapped) or only refuse when the wrapper's argument itself resolves to an Annotated.

_{Source: code-review · Claude Code (model not exposed)}

Generated by Claude Code

cipherstash-agent · 2026-11-10T00:42:46Z

+    names = _library_field_names(cls)
+    from_hints = names is None
+    if names is None:
+        names = [name for name, hint in hints.items() if not _is_class_level(hint)]


minor On the type-hints path every non-ClassVar annotation is a field, so a SQLAlchemy class with __tablename__: str = "users" or __table_args__: tuple = (...) raises declaration_no_marker for a name the user cannot mark. Skip dunder names here.

_{Source: code-review · Claude Code (model not exposed)}

Generated by Claude Code

cipherstash-agent · 2026-11-10T00:42:48Z

+def _read_fields(cls: type) -> List[DeclaredField]:
+    try:
+        hints = get_type_hints(cls, include_extras=True)
+    except (NameError, TypeError) as error:


minor Only NameError and TypeError are caught, so a forward ref like "models.Post" with a missing attribute (AttributeError) or a malformed string (SyntaxError) escapes the class statement as a raw exception with no field and no declaration_hints_unresolved code. The same gap is in _unresolved_field at lines 179 and 190.

_{Source: code-review · Claude Code (model not exposed)}

Generated by Claude Code

cipherstash-agent · 2026-11-10T00:42:50Z

+import pathlib
+
+_CASES = pathlib.Path(__file__).with_name("_declaration_cases.py")
+exec(compile(_CASES.read_text(), str(_CASES), "exec", dont_inherit=True), globals())


minor The cases are exec'd from source text, so pytest's assertion rewriting does not apply: a failing assert planned(User) == EXPECTED_FIELDS reports a bare AssertionError with no values. Import _declaration_cases after pytest.register_assert_rewrite, or compile the rewritten AST.

_{Source: code-review · Claude Code (model not exposed)}

Generated by Claude Code

cipherstash-bot

Recommendation: 🟡 merge after changes

Three changes before merging: fix the wrong field name in declaration_hints_unresolved for ClassVar fields, and add tests for _is_class_level and for refused Mapped[...] columns. Nothing else blocks the merge.

Other findings not posted as comments

Optional: no test declares a decorated subclass, so base-class fields are not checked in the plan. languages/python/python/stackencrypt/_declaration.py:123
Optional: no test declares Annotated[Mapped[str], TextEq] on a dataclass, so the from_hints=False rule is not pinned. _declaration.py:258
Optional: no test puts the unresolved hint on an undecorated base class, so the MRO walk in _unresolved_field is not checked. _declaration.py:176

How this review was made

Agent	Model	Review type	Result
claude	claude-opus-5-5	test-gap	3 found, 2 posted
claude	claude-opus-5-5	rust	0 found, 0 posted
claude	claude-opus-5-5	python	1 found, 1 posted
claude	claude-opus-5-5	infracode	0 found, 0 posted
codex	gpt-5.6-terra	test-gap	1 found, 1 posted
codex	gpt-5.6-terra	rust	0 found, 0 posted
codex	gpt-5.6-terra	python	0 found, 0 posted
codex	gpt-5.6-terra	infracode	0 found, 0 posted

Synthesis: claude-sonnet-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 1 posted finding(s) were raised by two or more models.

Plain language: the reader test did not run; only the word-and-shape check was applied.

Stack: position 5 of 15 (#1405, #1406, #1414, #1415, *️⃣ #1433, #1437, #1453, #1499, #1509, #1510, #1511, #1513, #1516, #1517, #1582). *️⃣ marks this pull request.

Context loaded: the description, 3 linked issue(s) and 7 discussion entries.

cipherstash-bot · 2026-11-10T04:07:38Z

+
+            probe.__annotations__ = {name: hint}
+            try:
+                get_type_hints(probe, global_names, dict(vars(base)), include_extras=True)


Change before merge: _unresolved_field can name a valid ClassVar field as the field that does not resolve.

Impact: The error names the wrong field, so the developer looks for the mistake in the wrong place. This happens on Pydantic models and on SQLAlchemy classes, whose base classes have string ClassVar annotations. It also happens for a user's own ClassVar field in a module that uses from __future__ import annotations.

Evidence: probe is a function, so get_type_hints reads each string hint as a function argument. A function argument cannot be ClassVar. I ran get_type_hints on a function with the annotation 'ClassVar[int]'. It raised TypeError: typing.ClassVar[int] is not valid as type argument. The except (NameError, TypeError) on line 190 then returns that field name. The loop walks the MRO from object downwards, so a base-class ClassVar is probed first. The test test_an_unresolved_hint_is_refused_with_its_field has no ClassVar before the bad field, so it does not find this.

Fix: Probe with a class, not a function. get_type_hints then accepts ClassVar as a class annotation. Add a test with a ClassVar field before an unresolved field, and one with a Pydantic model.

probe = type("probe", (), {"__annotations__": {name: hint}}) get_type_hints(probe, global_names, dict(vars(base)), include_extras=True)

_{Found by 1 model: claude}

cipherstash-bot · 2026-11-10T04:07:39Z

+    return None
+
+
+def _is_class_level(hint: Any) -> bool:


Change before merge: no test reaches _is_class_level, so the ClassVar and InitVar filters run in no test.

Impact: _is_class_level only runs on the type-hints path, for a class with no library field list, such as a SQLAlchemy class. If either check breaks, a ClassVar or InitVar on such a class raises declaration_no_marker, and no test fails.

Evidence: test_class_level_names_are_not_fields in tests/_declaration_cases.py uses @dataclasses.dataclass. There, dataclasses.fields already leaves out the ClassVar, so _read_fields never calls _is_class_level.

Fix: Add a plain class that uses the type-hints path, with each form the function checks.

def test_class_level_hints_are_not_fields_on_a_class_read_from_its_hints(): @stackencrypt.model("users") class User: email: Annotated[str, TextEq] registry: ClassVar[dict] = {} bare: ClassVar = 0 token: dataclasses.InitVar[str] = "" assert planned(User) == [("email", TextEq)]

_{Found by 2 models: claude, codex}

cipherstash-bot · 2026-11-10T04:07:39Z

+        raise _no_marker(name)
+    # One wrapper around Annotated is read by structure: Mapped[Annotated[...]].
+    unwrapped = False
+    if get_origin(hint) is not Annotated and _is_wrapper(hint):


Change before merge: the Mapped[...] unwrap step has tests only for correct fields.

Impact: For a SQLAlchemy class, rule A runs after the decorator reads through Mapped. No test checks that Mapped[str] with no marker is refused, or that two markers inside Mapped[Annotated[...]] are refused. A change to the unwrap step could let a column with no marker into the plan, and no test would fail.

Evidence: The refusal tests in tests/_declaration_cases.py use dataclasses or plain classes. The only Mapped refusal test is Annotated[Mapped[str], TextEq] (line 177).

Fix: Add refusal tests on a real mapped class. Expect declaration_no_marker for Mapped[str] and declaration_two_markers for Mapped[Annotated[str, TextEq, Passthrough]], with error.field == "email".

_{Found by 1 model: claude}

yujiyokoo mentioned this pull request Oct 29, 2026

python: declaration reader and rule A (model decorator) #1386

Open

5 tasks

yujiyokoo force-pushed the feat/python-sdk-declaration-reader branch from 905205f to 2d76b18 Compare October 29, 2026 07:06

yujiyokoo added this pull request to stack #1520 November 4, 2026 02:58

yujiyokoo and others added 2 commits November 5, 2026 00:17

refactor(python): use lower_snake_case error codes and reasons (#1386)

bac90dd

Codes such as `declaration_no_marker` and the reason `rule_a` follow the amendment on #1341. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

yujiyokoo force-pushed the feat/python-sdk-declaration-reader branch from 2d76b18 to bac90dd Compare November 4, 2026 15:24

yujiyokoo mentioned this pull request Nov 9, 2026

feat(ci): tests-python.yml for the Python SDK workspace #1406

Open

yujiyokoo marked this pull request as ready for review November 10, 2026 00:40

yujiyokoo requested a review from a team as a code owner November 10, 2026 00:40

cipherstash-agent reviewed Nov 10, 2026

View reviewed changes

This was referenced Nov 10, 2026

feat(python): walking skeleton for the Python SDK #1405

Open

test(python): verify the two engine facts the spec depends on (#1384) #1414

Open

cipherstash-bot reviewed Nov 10, 2026

View reviewed changes

Navigation Menu

Repository navigation

feat(python): read a model's declaration and check rule A (#1386) - #1433

feat(python): read a model's declaration and check rule A (#1386)#1433
yujiyokoo wants to merge 2 commits into
feat/python-sdk-eql-markersfrom
feat/python-sdk-declaration-reader

yujiyokoo commented Oct 28, 2026

Uh oh!

changeset-bot Bot commented Oct 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading

Review skipped

Uh oh!

cipherstash-agent left a comment

Uh oh!

cipherstash-agent Nov 10, 2026

Uh oh!

cipherstash-agent Nov 10, 2026

Uh oh!

cipherstash-agent Nov 10, 2026

Uh oh!

cipherstash-agent Nov 10, 2026

Uh oh!

cipherstash-bot left a comment

Uh oh!

cipherstash-bot Nov 10, 2026

Uh oh!

cipherstash-bot Nov 10, 2026

Uh oh!

cipherstash-bot Nov 10, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yujiyokoo commented Oct 28, 2026

Summary

Changes

Verification

Related

Review notes

Uh oh!

changeset-bot Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

coderabbitai Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

cipherstash-agent left a comment

Choose a reason for hiding this comment

Review attestation

Uh oh!

cipherstash-agent Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

cipherstash-agent Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

cipherstash-agent Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

cipherstash-agent Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

cipherstash-bot left a comment

Choose a reason for hiding this comment

Recommendation: 🟡 merge after changes

Other findings not posted as comments

Uh oh!

cipherstash-bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

cipherstash-bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

cipherstash-bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot Bot commented Oct 28, 2026 •

edited

Loading

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading