Skip to content

Use message owners for heartbeat liveness and reply eligibility - #2630

Merged
10 commits merged into
mainfrom
next/response-policy-owner
Oct 23, 2026
Merged

10 commits merged into
mainfrom
next/response-policy-owner

Conversation

@trissim

@trissim trissim commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

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:

  • The old newest-answer query hit its 1,000ms cutoff without a result; the joined/lazy saved-range case completed in 20.72ms with one original decode.
  • The original one-row conversation UNION used 256,859 SQLite instructions on the long sample versus 6,050 on the short one. Bounded constituent queries use 50 on both. This is metadata-query work, not end-to-end UI latency.
  • Direct lookup REGRESSES against the actual original cached path. On four identical saved-range 100-reply selections, actual pre-change received(source=DirectReplySource) costs 52–105ms and 100 full delivery decodes. Current direct_replies costs 101–218ms, 170–601 Message reads and 100 full delivery decodes: roughly 1.3–4.2x slower in those samples. Both return identical sequences. The earlier 988→363ms comparison only beat a rejected full-manifest replacement; it was not the original baseline. Direct scanning and the constant-cost target remain open with this worker.

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.

@trissim
trissim force-pushed the next/response-policy-owner branch from e0cfac6 to 24d5887 Compare October 23, 2026 04:41
@trissim
trissim changed the base branch from next/commander-control-plane to integration/installer-report-lifetime October 23, 2026 04:41
@trissim
trissim force-pushed the next/response-policy-owner branch from 7d64bbc to 4cc95af Compare October 23, 2026 05:00

@trissim trissim left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

@trissim trissim changed the title Derive reply eligibility from the original message Use message owners for heartbeat liveness and reply eligibility Oct 23, 2026
@trissim
trissim changed the base branch from integration/installer-report-lifetime to main October 23, 2026 06:10
@trissim trissim closed this pull request by merging all changes into main in b2b0358 Oct 23, 2026
@trissim

trissim commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

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.

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.

1 participant