Skip to content

Finish table mutation rule ownership (supersedes 2471) - #2510

Merged
trissim merged 39 commits into
mainfrom
unit-ownerexit/p429-table-rules
Oct 22, 2026
Merged

trissim merged 39 commits into
mainfrom
unit-ownerexit/p429-table-rules

Conversation

@trissim

@trissim trissim commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

P429: declaration-owned table mutation rules

Current queue-conflict repair — supersedes head/readiness wording below

  • Published head f81cf0186afa7577e39050b4af9084dfcdb9f181, push and ls-remote matched; worktree clean. It merges main d3fcbd66216483f8c71baf377a677b047f21ceaa. GitHub reported the previously accepted d5921a7 CONFLICTING, so the instructed unchanged-head enqueue was not possible. The sole conflict was typed_table.py's imports: retained LifecycleState and main's RegistryFile/MaintenanceBarrier imports. No new implementation change or formatter run.
  • fork-review-ad accepted d5921a7, running all 16 touched files independently: 314 passed, 4 failed; all four failures reproduced on main afb5909 (two Virtual bus batch: W, B1, B2, C, D and E integrated #2549 baseline defects and two runtime-prefix guards). That verdict is historical, not approval of f81cf01. A fresh head verdict is requested; PR remains draft, not queued.
  • Own post-merge run at f81cf01: four affected modules, 115 collected/executed: 114 passed, 1 failed, 68.78s, no setup/teardown errors or deselections. Both awareness corruption tests pass. Failure: test_normal_rename_does_not_inject_old_selected_claim_into_new_owner, the existing OmittedAwareness.omitted_count defect owned by Virtual bus batch: W, B1, B2, C, D and E integrated #2549. Command: PYTHONPATH=src timeout 300 /home/ts/code/projects/agent-comms/.venv/bin/python -m pytest -n 0 -o faulthandler_timeout=60 --basetemp=/home/ts/.cache/agent-scratch/unit-ownerexit/p429-queue-merge-test-data tests/test_table_mutation_capabilities.py tests/test_coordination_store.py tests/test_optional_awareness_projection.py tests/test_maintenance_barrier.py. Log: p429-queue-merge-tests.log.
  • Fresh measurement against captured origin/main 46391711d9a439eecd2aec0fa56c54bbc65227c7: src +1479/−1647 = −168 net, 3126 changed lines / 9970 tokens; tests +560/−114 = +446 net, 674 lines / 1341 tokens. Combined 3800/11311 = 0.336, about 1.13× Derive table mutation guards from capabilities #2471's 0.296. Main advanced to The commander and its forks accept a reviewed PR head in one verdict #2599 after the merge, so this comparison base is newer than the included main; the raw diff includes that unmerged-main difference, not only P429 edits. Measurement artifact: p429-queue-measure.json.
  • Existing refactor-audit Repository/measure_source inventory at f81cf01: 441 src / 474 test modules, zero parse omissions (p429-queue-audit.json). This is syntactic evidence, not dynamic binding or behavioral proof.
  • Ordered whole-src BEFORE-trigger check: 0. Derive table mutation guards from capabilities #2471 ancestry and the separate ordinal commit remain; formatter merge 505a55c remains absent. No native/runtime/UI qualification claimed. The UNKNOWN d592 delivery operation has not been replayed, and no new delivery claim is asserted.

Preserves #2471's four implementation commits: f251e0299 is an ancestor; formatter merge 505a55c77 is not. No formatter, local ratchet or full suite was run.

Owners and deletions

  • Replaces 28 per-table trigger methods with mutation capabilities composed through the row MRO. RowMutation.abort_trigger owns the event template (IMPL-12).
  • FrozenColumn(Column) owns immutability; TypedTable derives the aggregate guard. Deleted all 11 production immutable-column string rosters, preserving all 56 fields, including generated WakeAssignment.wake_mode (MEMB-2/BOUND-7).
  • Deleted _immutable_assignment_matches; cohort acceptance uses WakeAssignment's existing acceptance check.
  • ExecutionReference, RetryPartition and ReplayAssessments live with ExecutionRecord. Discovered imports migrated without compatibility names/local replay imports. Deleted redundant frozen enum coercion; moved database methods are typed.
  • ReplayState owns revision checks, execution membership, authorization, revocation and recording. NoReplayAssessment/ReplayAssessments implement their differences; their parent owns observation. Deleted replay-absence decisions in the readers, snapshot and mutation consumers. Existing UnionDecoder/NullMember retain row-or-JSON-null representation; no alternate codec/store (IDEN-3/IMPL-4).
  • CohortDeliveries retains the durable SQL identity cohort_delivery_receipts.
  • One OrdinalRule.increment drives Python prediction and SQLite guards. Repeated attempt-reference + 1 is gone; ordinal work remains separate commit 49618f2a4.
  • Consumed Durable stores derive their declared schema on the writer's open #2578. Deleted the TableTriggers carry and its two tests (188 lines) and kept main's deletion of carry_declared_schema.py. Deleted CoordinationStore's duplicate initializer; its normal constructor calls CoordinatorTable.reconcile_schema (IMPL-5/TIME-1). No wrapper or second migration path.
  • Recovery fixtures now receive the child's actual announcement through a one-way pipe, whose closure reports a failed producer. Deleted both independent ten-second Queue readiness clocks. Existing leave() owns shared process cleanup; no production timeout/cap/prefix guard changed.
  • Send-back repair: corruption tests derive guards from TypedTable.triggers(), not retired trigger names (MEMB-4). Shared test context restores the declared guards before checked awareness reads. Native-journal and sidecar damage tests also consume declared guards. No production compatibility names restored. The expanded pass also found a missed ParticipantSnapshot.owner_thread test consumer; it now reads owner_identity.owner_thread. Claim-bus fixtures reuse the existing root-lifetime owner, joining writers before TemporaryDirectory removes their root. Restored the unrelated annotationlib line wrapping to main's exact layout.

Prior repair checkpoint and measurement

Repair checkpoint d5921a7ba726477adb920a642f80ce765f2e6ddb, push/ls-remote matched.
Included immutable main/base 954534a8e170ead817973e1c1f0316f8891b37b8.

Tree Added Deleted Net Changed lines Changed tokens
src 1452 1540 -88 2992 9374
tests 553 109 +444 662 1310

Tokens: whitespace-delimited added/deleted words from git diff --word-diff=porcelain against that captured base. Combined 3654 / 10684 = 0.342, versus preserved #2471 2573 / 8684 = 0.296: 1.15x, not twice it. Moves are counted.

Verification — exact limits

  • Correction: the earlier seven-module c5d125b batch executed 201 cases (200 passed, 1 failed), but it did NOT cover every touched test. Review reproduced two additional failures in test_optional_awareness_projection.py: both referenced retired trigger names. That omitted coverage invalidated the claim that cleanup was complete. The two tests and related damage consumers are repaired at fe4b7e9; the expanded 16-module run at fe4b7e9 completed 313 tests: 304 passed, 9 failed, plus 2 teardown errors, in 204.33s. This exposed two missed ParticipantSnapshot test accesses (repaired in d5921a7), a temporary-root cleanup race (also repaired), a runtime-only metadata case stopped by the prefix guard, the two baseline failures reported by review-s and already repaired on Virtual bus batch: W, B1, B2, C, D and E integrated #2549, a bounded-awareness failure, and two fixture child-exit failures with teardown errors. All are retained in p429-guard-tests.log; they are not hidden behind the earlier 200/201 result. Final single-worker run of the six repaired/affected modules at d5921a7: 97 collected, 92 executed: 91 passed, 1 failed, 5 native-only cases explicitly deselected, zero setup/teardown errors, 217.47s. Full awareness module: 14 passed, 1 failed; both reviewed corruption tests pass. All 25 selected recovery-fixture cases, 18 native-journal data tests, 28 sidecar tests, four ACP owner tests and two claim-bus tests pass. The one remaining failure is test_normal_rename_does_not_inject_old_selected_claim_into_new_owner, the same failure review-s reproduced at main 3754641. Bus B1 reports the Virtual bus batch: W, B1, B2, C, D and E integrated #2549 fix passes at 996765d; that unmerged batch is not consumed here. No compatibility member was added to OmittedAwareness and no assertion, timeout, cap or prefix guard was weakened. Log: p429-guard-final-tests.log. These results are separate runs, not additive pass totals.
  • The earlier seven-module run's native failure was test_launch_recovers_before_reserving_replacement_pid: fixture preparation now qualifies through merged P447: keep source-only backend qualification independent of native launch #2571, but the real launch correctly refuses this worktree artifact outside the running interpreter's installed runtime prefix. Stack: PrivateNkLaunch.installed -> VerifiedNativePackage.acquire -> NativePackage.require_runtime. PublicationActivationBlocked preserves that cause. Fixture reported 0 localhost posts, 0 native input proofs, no children. This is NOT a successful native launch or clean all-green batch. Matching-runtime installation/launch qualification belongs to the merger; no guard weakening or fake prefix here.
  • Historical schema-consumer batch: 196 passed, 1 failed, 3 setup errors. Replaced the obsolete index-drift-refusal test with unexplained-column refusal plus symlink protection, and removed the fixture readiness clock. Earlier replay checkpoint: 191 passed with one explicit exclusion. Neither was complete touched-file coverage.
  • Real normal-open proof on the retained 166,331-row / 25-table copied coordinator: deliberately replaced its DELETE guard with an old guard; ordinary CoordinationStore construction repaired it through TableSchema. All table counts and SHA-256 digests unchanged; zero row writes, no FK violations; raw SQL DELETE refused. Second ordinary open made zero DDL changes (SQLite schema_version unchanged). No carry/ledger invoked. p429-schema-normal-open-proof.log records source head 5ed7eca; production schema/connection implementation is unchanged at the current head.
  • Prior replay proof: all 1,237 retained replay rows plus absence wire-round-trip, zero writes/FK errors. Original production stores/sessions were not altered.
  • Whole-source ordered check at this head: git grep -nE 'BEFORE (UPDATE|DELETE|INSERT)' -- src ':!src/agent_comms/typed_table.py' | wc -l => 0.
  • Guard-consumer source pass used existing refactor-audit Repository/Package: 440 src modules and 472 test modules, zero parse omissions, at fe4b7e9. Final consumer inventory at d5921a7 parses 441 src modules and 474 test modules, zero omissions, after consuming the newer main; artifact p429-guard-final-audit.json. The whole-tree DROP TRIGGER search found and migrated the two awareness consumers, the native-journal consumer and the sidecar consumer; remaining production-derived drops already use declarations. Artifacts: p429-guard-consumers-before.json and p429-guard-consumers-after.json. Earlier Repository/measure_source parsed 431 Core modules, zero omissions, before/after schema-consumer integration. Targeted AST inventories retain replay declarations/bases/consumers and both readiness producers/fixtures plus leave(). Earlier paired Toad/Textual searches found no relevant imported capability consumers. Name matching does not prove dynamic receiver binding; ambiguous/dynamic references require semantic reading. Inventories of different node granularity are not a numeric reduction claim.
  • Explicit shared-Python native preparation/update-pins and subsequent normal preparation succeeded. Not an installed application claim. No whole-tree mypy. Current d5921a7 readback records five hosted checks successful; install-rehearsal was still running. That is not live acceptance.
  • Resource helper /home/ts/bin/agent-resource-check is absent; direct measurements before the expanded run: 36 GiB disk free, 3.0 GiB RAM available. Final checks use one worker. Existing shared interpreter/dependencies; persistent owned scratch; no new venv/clone/sync.

Delivery and remaining installed acceptance

Core linked #2510 with order pr P429 --existing 2510. Commander 17183 disposed the older e7c6cd8 delivery timeout: no claim landed, do not recover it, record the newest qualified head fresh. The later coordination bus-note timeout was not replayed.

PR is back in draft after review found the missed awareness failures. The c5d125b claim was successfully recorded through comms_order_deliver at revision 92, then sent back; the CLI recording blocker is no longer current. Repair checkpoints fe4b7e9 and d5921a7 are published; full completion/readiness is not claimed. Both tests named in the send-back pass. The known rename-awareness failure remains with #2549's existing implementation/integration owners; matching-runtime native acceptance remains with the merger. The old seven-module result was not complete touched-file coverage, and this report no longer says it was. A failed Core progress-update command was not replayed.

The remaining native failure needs the merger's matching installed runtime, then the actual saved-session/native/Toad journey. The copied-store workflow works; this branch is not claimed installed or live-verified. Source delivery does not close the installed acceptance gap.

Assumed:

  • Commander 16111 permits forced person.owner_thread -> person.owner_identity.owner_thread in native_source_cursor.py. Other cursor edits remain the ordered renames/import replacement, no layout churn.
  • Commander 16478/16806 requires replay-state ownership despite the earlier relocation-measurement challenge. No ratchet edits.
  • The current replacement order supersedes discarded capability-folding work.
  • Fresh-environment installation and matching-runtime native/saved-session/Toad qualification belong to the merger; worker copied-store/SQL proof is not a substitute.
  • Derive table mutation guards from capabilities #2471 remains open until this PR lands its preserved commits.

@trissim
trissim force-pushed the unit-ownerexit/p429-table-rules branch from 948e5ec to e9d3b37 Compare October 21, 2026 18:00
@trissim
trissim marked this pull request as ready for review October 21, 2026 22:00
Copilot AI balanced review requested due to automatic review settings October 21, 2026 22:00
@trissim
trissim enabled auto-merge October 21, 2026 22:00

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 reached their quota limit.

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 disabled auto-merge October 21, 2026 22:05
@trissim
trissim marked this pull request as draft October 21, 2026 22:05
@trissim
trissim marked this pull request as ready for review October 21, 2026 23:41
@trissim
trissim enabled auto-merge October 21, 2026 23:41
@trissim
trissim marked this pull request as draft October 22, 2026 00:02
auto-merge was automatically disabled October 22, 2026 00:02

Pull request was converted to draft

@trissim
trissim merged commit bd925bc into main Oct 22, 2026
6 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