Repository navigation
Preserve nominal ID kinds through input, turn and storage owners - #2943
Merged
Merged
Conversation
trissim
marked this pull request as ready for review
October 23, 2026 22:32
There was a problem hiding this comment.
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.
|
debt-tools at 9eb631a, every finding: |
|
debt-src at 82ccab9, every finding: |
This was referenced Oct 23, 2026
…o fix/producer-digest-value-types
trissim
changed the title
Use a canonical resource path in the wake carry fixture
Preserve carry refusals and rewrite guarded rows through their declarations
This was referenced Oct 24, 2026
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.
Core ids lost their kind when decoded as plain str. Introducing nominal values exposed the other half of that defect: string-typed SQL readers and exact-str guards rejected ids that the producer had already minted correctly. This batch retains the kind through the producing fields, ingress, storage readers, turn state and native-send owners.
Current head: a953c92. Remote equals HEAD. No main merge or enqueue requested.
Final decoder declaration
AssignmentResponse.from_columns is the actual nullable SQL entry: it decodes the three columns, returns UnansweredResponse when all are NULL, constructs AnsweredResponse when complete, and refuses a partial row. notification_assignment.py now declares it with @boundary_decoder directly under @classmethod. Only that import and decorator changed in the final commit; the decode and turn_input_source.py are untouched.
The commander’s review fork reported 1052 passed on parent 07e83c6 across 88 affected files, with only the two failures also present on main. All 28 earlier branch failures passed there. This unit ran no tests.
Requested local comparison on the current committed head, using two workers:
{ "introduced_lines": { "NoneAbsence:src/agent_comms/turn_input_source.py": [ 48, 90 ] } }Its only positive delta is the explicitly approved NoneAbsence:src/agent_comms/turn_input_source.py +1. Exit 1 is solely that approved result. No baseline, exemption, pragma or decoder rewrite was added.
Value and storage ownership
NominalText inherits its spelled_by declaration. NominalSpelling records the spelling once; the declared decoder constructs the field's own kind and refuses sibling kinds with MalformedRecord after checking spelling. WireValue and NominalText answer representation() themselves. Id128 owns the UUID mint; ClaimId, AssignmentGeneration, TurnId, GoalId, RootId, ThreadLookup and InputId inherit it without per-kind spellings or mint copies. MessageId owns the unchanged twelve-hex digest. The test pins three old-main hashes and the existing wire strings. The temporary plain-str allowance names RedecodesDeletedPhase.
Mints move from Id128Text/MessageIdText to values, including default_factory. CompactionPhase owns the compaction fact; its id has no compaction prefix. The old thread_identity.TurnId wrapper is RegisteredTurn here; its complete removal and recorded-context/task carry are in separate draft #2965.
The duplicate agent_backend.InputId is deleted. Input producers declare InputId and consumers no longer box/unwrap it. MessageReference and message publication/response fields declare MessageId. TypedRow readers decode those declarations once at SQL entry. TextStorage supports the nominal representation through its existing owner; producer digest includes the value declarations. These paired changes came from #2945/#2966, folded into this branch. Further PRs into this branch wait for the commander's review verdict.
Turn and send ownership
TurnState holds active: TurnDeclaration and finished: TurnRecord. NoObservedTurn/TurnFence own the runtime observation; the old optional observed id and finished_turn_id field are deleted. Required queued input ids come from PromptRequest's mint, with no second None/default mint.
TurnInputSource and OwnedTurn require TurnAcceptance. AcceptedTurn owns its queued acknowledgement, current-input check, start update and removal; NoTurnAcceptance leaves the queue alone. Controller | None is retained honestly, per the commander's explicit exception. Scheduled wakes, startup input and selected originals explicitly choose NoTurnAcceptance; queued inputs pass AcceptedTurn.
InputStartListener lives in agent_backend. ReportInputStart/from_callback and native_start callback arguments are deleted. SendBoundary is an ABC with send_original/send_following/send_interrupt; OwnedSend composes it with InputStartListener and CodingToolOwner. Permission before write and acknowledgement after commit have distinct names. TurnInputSource owns shared preparation once. The callable alias, interrupt_boundary field and lambdas are deleted. Direct native sessions preserve the existing unrestricted case through NoSendBoundary.
InputForwarding retains its original typed InputRequest in the one pending index; the RPC channel already tracks the wire command. Start matching returns zero or one InputId instead of a bool plus an optional id.
Latest read fixes: one commit, items 1–8
All eight ordered fixes and their affected fixture producers are in 2818c2d, directly on reviewed parent a1f4ffb.
The constructor inventory found six WakeAssignment fixtures still passing label strings; test_coordination, test_coordination_store and test_coordination_nominal now mint MessageId at those constructors. historical_source in test_native_context_reference also mints its source message id. These are producer changes, with no store-value wrapping or consumer re-decode.
Readers inspected and left unchanged
AST inventory: 961 production/test/tool modules, zero parse omissions; immutable skill fixtures excluded. Known constructors and the relevant attribute consumers were read semantically. External callers and ambiguous dynamic invocation are outside this inventory.
Source checks and lint
B, C and D from the order print nothing. A's literal command still prints two legitimate pre-existing lease checks:
The missing stored-turn lookups, reports_turn("same-assistant-turn") and owner.claimed_generation are gone. The two real lease checks are preserved rather than hidden to make an overbroad grep green; the commander has this exact output.
Only changed lines/import blocks fixed. No formatter, tests, suite or ratchet run. The five changed source modules import successfully. The review fork runs the affected paths.
Stored claim carry remains separately owned
WakeClaim is version 2 because its nested RecordedScope.turn_id schema became stricter. RecordedScope itself has no version. The existing carry family contains WakeClaimTurnIds, intended to validate v1 ids and rewrite the v2 record; no schema override, substitute id or silent skip is used. The commander owns the remaining carry-walk failure. This latest commit touches neither the tool, its fixture nor field_codec.py/typed_table.py. The reviewed fixture-path follow-up #2967 is already in parent a1f4ffb.
Assumed: nominal values keep their existing wire spellings; raw CLI/tool/SQL data decodes at its actual entry; nullable published-response columns remain the existing external unit; real lease-generation checks are not the deleted invented Thread attribute; tests stay with the review fork. No current-head test pass or live-install qualification is claimed.
git diff --stat origin/main...HEAD