Skip to content

Compare a stored record's required keys as a set - #3621

Merged
trissim merged 2 commits into
mainfrom
fix/stored-form-required-set
Oct 29, 2026
Merged

trissim merged 2 commits into
mainfrom
fix/stored-form-required-set

Conversation

@trissim

@trissim trissim commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

UncarriedRecordFormat (tools/debt_ratchet.py:2669) compared StoredForm by dataclass equality, and required was a list, so reordering the required keys counted as a format change. Declaring name/created_at in a parent class reorders FieldCodec's schema list without changing a single stored byte. #3607 went red on exactly that for Thread.

  • StoredForm.required is now a frozenset: the owner says the order means nothing.
  • wire()/from_wire() are the one JSON boundary. The base revision's older probe still prints a list, and from_wire reads it as a set, so this works against current main bases immediately.

Proof: real records from agent_comms.active_route round-trip equal, and equal again with required reversed.

parent class declares a field
nothing on disk changes
ratchet: format change, carry it
carry what

🤖 Generated with Claude Code

UncarriedRecordFormat compared StoredForm.required as an ordered list, so
declaring a field in a parent class (which reorders the schema's required
list without changing what is written) counted as an uncarried format
change. #3607 hit it on Thread. The required keys are a set: StoredForm
owns that, and both revisions' probe output is read through from_wire.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnH6dR85kyFKcc2tQAJLhK
Copilot AI balanced review requested due to automatic review settings October 29, 2026 22:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim
trissim enabled auto-merge October 29, 2026 22:07
@github-actions

Copy link
Copy Markdown

debt-tools at c85517c, every finding:

::warning file=tools/debt_ratchet.py::LintFindings +1 (warns, never blocks)
grew RawRecordShapes:tools/stored_formats.py +1
grew StringSubscript +3
found StringSubscript at tools/stored_formats.py:46 tools/stored_formats.py:46 tools/stored_formats.py:46

The previous head read the probe's JSON through from_wire with string
subscripts, which the ratchet rightly counted. StoredForm's __post_init__
now turns whatever sequence it is given into the frozenset, the probe
serializes it sorted, and the ratchet constructs it as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnH6dR85kyFKcc2tQAJLhK
@trissim
trissim added this pull request to the merge queue Oct 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 29, 2026
@trissim
trissim added this pull request to the merge queue Oct 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 29, 2026
@trissim
trissim added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit f2acc0c Oct 29, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants