Repository navigation
Include declared value and spelling classes in producer digests - #2945
Merged
trissim merged 8 commits intoOct 24, 2026
Merged
Conversation
trissim
marked this pull request as ready for review
October 23, 2026 22:46
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.
trissim
marked this pull request as draft
October 23, 2026 22:48
trissim
marked this pull request as ready for review
October 23, 2026 22:50
|
debt-src at 76b6605, every finding: |
trissim
marked this pull request as draft
October 23, 2026 22:54
trissim
force-pushed
the
fix/producer-digest-value-types
branch
from
October 23, 2026 22:57
24c16e8 to
7c35a8c
Compare
…o fix/producer-digest-value-types
trissim
marked this pull request as ready for review
October 24, 2026 00:31
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.
MessageIdentitySource now declares its message_id as MessageId in production. Its digest therefore includes MessageId's minting code and MessageIdText's spelling: changing those implementations invalidates stored rows instead of silently retaining old identities.
Ready, based on #2943 (
refactor/id128-nominal-type). Merged Reviewer head0cf0835f782aaededa4f532163cdeeb9083516f9into headd1bb4bccaac7b2a3510e23b76406b81aa2ddab81; both land as one batch. TextStorage remains in this PR. The merge was clean; none of the ten reserved send-back files was resolved or edited by Drafts.Changes
message_reference.py:23: the actual inherited column ismessage_id: MessageId.field_codec.py:1855: the existing classifier is now the publicFieldCodec.representationclassmethod. All internal callers and both typed_table callers use it; the old private method is deleted, with no alias.typed_table.py:181–188: TextStorage asks that owner for the declared text representation. Nominal IDs retain plain TEXT storage; JSON storage does not receive this column.typed_table.py:2092–2123: column value/representation owners come from declarations. The MRO walk includes only FieldRepresentation or NominalText descendants (WireValue already inherits FieldRepresentation), retaining each codec-family root and excluding unrelated Python/stdlib/C ancestors. No module-name filter or hand-written owners.typed_table.py:2111: only dataclass declarations read physical columns. BusSchema, BusProjectionTables and ActivityMaximum all answer producer_owners without accessing nonexistent dataclass parameters (4, 3 and 4 owners respectively).tests/test_bus.py:135: deleted the declaration rewrite. The test copies the real production source, changes only MessageIdText.digits, then separately an unrelated GitCommitText spelling.tests/test_bus.py:149: the new test creates the real MessageIdentitySource table, inserts a MessageId, asserts SQLite typeof is text and the stored value is exactly its plain string, then reads an equal row with type(message_id) is MessageId.Real-tree digest proof at 7c35a8c, before this parent sync
Fresh interpreters ran the canonical producer family on source copies of that pushed branch head. No declaration is rewritten in the test or proof.
cff8ffeee1974c7da4586bc483adb0950fe294c3ad61ddff8a0fc64b34997b1e74c3297a00dce1198542c75f9ba02775f7b136cd2d17940f2503b37142c9b2eacff8ffeee1974c7da4586bc483adb0950fe294c3ad61ddff8a0fc64b34997b1eOwner-set changes versus the #2943 parent
The canonical producer_stamp import registers 11 concrete tables on both pinned trees, rather than the 13 from the earlier review inventory. These are every table returned by ProducedTable.members_with(ProducedTable) on those trees, not a selected subset.
answered_sourceawaiting_answer_sourcebus_activity_indexbus_display_indexbus_route_countsclaim_operationsheld_resourcesmessage_identity_sourceFieldRepresentation,MessageId,MessageIdText,NominalSpelling,NominalText,SpelledText,TextRepresentation,ValidatedScalarreceived_sourcesent_activitysent_sourceAll eleven digest values change once because ProducedTable itself is an existing hashed ancestor and its producer_owners implementation changes. Only MessageIdentitySource gains value/codec owners. The whole-class source hash remains; no manual version or narrowed hashing.
Focused checks at 7c35a8c, before this parent sync
Only the following two nodes ran (nice 19, timeout 300, -p no:xdist):
An earlier attempt on the prior #2943 head failed before collection at acp_extension.py's removed Id128Text.mint call. Syncing the parent's 6cef37d update supplied its InputId.mint migration; no ACP edit belongs to this PR.
Direct SQLite proof: stored
('9f72589d55b5', 'text'); decoded type MessageId; restored row equals inserted row. Abstract-base owner reads passed. AST parsing of all three changed production modules had zero omissions; the public query has eleven internal calls and two typed_table calls. Search of src/tests/tools finds no old private _representation reference. No suite ran.Assumed:
Fixture send-back fixed at d1bb4bc
The production type remains MessageId. Twelve free-text-ID call sites across the seven assigned test files now use MessageId.mint. This includes the additional single-quoted 'not-input' constructor at test_current_input_origin.py:275 which the requested double-quote grep missed. The three durable-reply references share one reply_id binding. Existing refusal/equality assertions remain; the absent-ID lookup still requires RowNotFoundError. No production file changed in this correction.
Requested grep:
AST traversal of all tests also checked single-quoted/keyword MessageReference message_id arguments and direct string lookup_message arguments: zero direct string arguments remain, hence zero malformed literals. AST parsed all seven edited test modules. This verifies the source migration, not the runtime results. No tests ran for this correction; commander review runs the two named nodes.
Ruff check ran on all 129 Python files returned by
git diff --name-only origin/main -- '*.py'. It reports 1,118 findings overall and zero on Drafts changed lines. Only the changed long lines were wrapped; no whole-file formatting or inherited debt cleanup.Assumed for this correction: deterministic minted values remain distinct from the actual stored messages in these fixtures; the three historical notifications intentionally share one reply identity; the single-quoted not-input site has the same migration requirement; runtime test results come from review, not from the AST/grep checks.
git diff --stat origin/refactor/id128-nominal-type...HEAD:git diff --stat origin/main...HEAD(includes #2943):