Repository navigation
Use message owners for heartbeat liveness and reply eligibility - #2630
10 commits merged into
Conversation
e0cfac6 to
24d5887
Compare
7d64bbc to
4cc95af
Compare
trissim
left a comment
There was a problem hiding this comment.
Bounded source and migration review accepted at 23e9e5b. Reply eligibility now comes from the actual message/policy owner; exact-reference answer reads keep greatest sequence and greatest timestamp separate. Ordinary conversation paging bounds each declared participant query before combining results. Both goal consumers use the shared direct-reply path, retaining frozen recipient/sender identities and UNKNOWN-input selection.
I independently ran the corrected three retirement tests and existing real predecessor-publisher journey: 4 passed in 7.85s. That journey covers the actual OwnerRequests → ReplyPolicy → BusHistory order, the pre-SQLite no-op, policy cleanup with real retained offset maps on main/archive copies, conversion retry, and normal after-acceptance retirement. The earlier unchanged reply-ordering/ordinary-page slices also passed in my eight-case review at 7d64. The rejected complete-catalog changes are removed; existing partial schema owners preserve HistoryOffset and optional private awareness. Unclaimed tables are preserved, and the general complete-file ownership gap remains explicitly open with the control-plane owner.
This is source/migration acceptance, not installed qualification or overall performance acceptance. The Message-first direct scan still examines up to 601 candidate messages for 100 replies. Its reported improvement is against the rejected full-manifest replacement, not the original cached DirectReplySource query; the original runtime baseline is being measured separately. The existing full projection carry still costs about 44 seconds on the retained bus. Rejected #2629 is excluded. Current machine-install repair and later installed native qualification remain with the integration owner.
|
The remaining implementation was superseded by #2636, merged at b2b0358. Its accepted head fcc1062 includes the awaiting-answer projection evaluated at append, bounded answer reads, heartbeat-family ownership, and the design-plan cleanup. GitHub already marks #2630 merged because its head is included in that history; there is no remaining standalone #2630 delivery. |
Commander replies could hide a silent heartbeat because the check selected the newest two-way DM before checking its sender. A service that never sent its first beat also escaped the check. The heartbeat repair reads the latest actual service message across both participants’ aliases and derives the first-beat obligation from the oldest open owner request.
Reply reads stored a computed policy and implemented its recipient rules again in SQL. AnsweredSource now joins actual complete question/reply references, orders grouped originals by the requested answer value, and evaluates the real message owner lazily. Greatest sequence and greatest timestamp remain independent selections. Unanswered pages apply eligibility before counting their limit. The copied policy column, recipients_sql/response_predicate implementation and DirectReplySource table/projection are deleted.
Direct replies read complete Message values through the existing decoder, use ResponsePolicy.starts_direct_turn, and materialize frozen Delivered records only for accepted candidates. ReceivedSource establishes the recorded recipient incarnation; sender identity and sequence cuts stay in the existing query. Goals._goal_input_review and RecordedGoalWait.has_reply use that path, preserving UNKNOWN input selection and cursors. Ordinary participant reads return actual Delivered values on a held snapshot. Conversation pages bound each declaration-derived ParticipantSource query before combining candidates; ascending order and explicit sequence selections retain their cuts.
The one-time tools/cutover/carry_reply_policy.py step validates the exact old table shape outside the incoming projection family, retires it, and runs the existing partial BusSchema carry. Its main installation witness and archive catalog lock cover the main root and catalog-declared snapshots. Backup/rewritten accounting names each affected file. Actual order is OwnerRequests, ReplyPolicy, BusHistory: before SQLite conversion the new step is a no-op; later history conversion creates current schema. With an existing conversion, the policy step preserves HistoryOffset until its existing readback/retry consumer and after_acceptance retirement finish. General complete-file ownership is still open in the plan; unknown tables are preserved, and the attempted full-catalog check is absent. No transient schema framework or permanent whitelist is added.
Validation on the corrected source: 45 focused cases pass in 19.62s. This includes the real predecessor publisher and worker conversion, main/archive old-policy retirement with exact retained offset pairs, transfer retry and normal after_acceptance map retirement; archive identity/catalog/read results stay unchanged. It also covers aliases, full reference identity, expired originals, newer ineligible candidates, eligibility before limit=1, non-monotonic timestamps, frozen participant reads and standby/UNKNOWN behavior. Independent review reran the corrected four conversion cases. A retained 19,189-record worker-process replay retired 8,290 cached rows, preserved all six durable source-table counts and content hashes, removed the old policy column, passed integrity/foreign-key checks and repeated as a no-op. That carry costs 44.02 seconds; rejected #2629’s projection skip remains excluded.
Performance measurements have different scopes:
Source trace of when direct lookup runs: explicit StandbyRequest.review uses Goals._goal_input_review, which pages through the outstanding tail in batches of 100. The measured 100-reply cost is therefore one page, not a complete-review bound. StandbyGoalAction.before_goal_publish also runs that review inside Goals._updating_goal's exclusive registry lock (goal_management.py:449; goal_actions.py:273). recover_closed_goal_wait and release_waits_after_terminal_turn likewise hold the exclusive registry lock around RecordedGoalWait.has_reply (goal_management.py:260,303; goal_waits.py:189). These automatic checks request limit=1, so the 100-reply samples do not measure their latency. InputDrain invokes recovery after a source observation while in standby; TurnRunner schedules release after terminal publication. Coordination.run_worker and the MCP tool offload keep this work off the event loop, but RegistryStore.reading_snapshot takes a shared lock (registry_store.py:300), so other registry readers/writers can still wait behind the exclusive holder. No per-frame query caller, measured lock contention, or demonstrated UI framerate effect was established. Scan/decode work under that lock violates the standing docs/DECISIONS.md rule and remains an open control-plane defect; no additional benchmark or implementation is included in this checkpoint.
NRA source evidence compares integration da502 with the corrected source: 956→958 modules, 5,388→5,391 original classes, 5,197→5,200 compact projections, 191 unprojected declarations retained, zero parse omissions. The selected ownership/consumer evidence includes the converter lifecycle. Compact projections and dynamic binding limits are source evidence, not behavioral proof. IDEN-5/IMPL-5 witnesses: the message role determines policy, RepliesDisplayScope.includes determines reply membership, and Delivered.direct_for determines direct delivery eligibility; the removed stored/SQL answers repeated those facts.
Receipts live under /home/ts/.cache/agent-scratch/commander-control-plane: final-source-before.json, final-source-after.json, reply-policy-final-upgrade-replay.json, ordinary-page-bounds-replay.json, ordinary-page-bounds-final-replay.json, answer-source-replay.json and direct-query-original-baseline.json. The last commit updates measured plan evidence only; src/tests/tools/hooks are unchanged from independently reviewed 23e9. This draft is outside the current machine-install batch. Installed native/UI qualification remains with the integration owner; no merged/installed or completed commander-phase claim.