fix(usecases): cheaper LCM drain yield and idle frontier init - #546
Conversation
Keep 5s/30s as omitted-deadline fallbacks only. Prove BudgetExceeded stays Partial and that a semantic miss does not take down exact, lexical, or graph.
|
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Cross-review from td automation (Plan 26). Crate-only / cheaper-path / Plan 23/26 typed. No edit, no merge, no stack flatten.
Crate-only: the intended change is 16f171893 (two usecases files). vs cursor/plan26-session-evidence-budget-backoff-9603 this PR also carries foundation + #539 query commits (2accc6160, 8e7be306c, 8fa116bd1, …). Restack so the GitHub diff is only usecases. Do not merge 421/509/539 into this PR.
projection_drain.rs — cheaper, typed, but the WARN can still loop. Yield-after-first SanitizationRefused/Contract skip is the right shape (less receipt construction, Deferred, not a publication receipt, not a deadline bailout). Same lesson as #538: skip is correct, the storm is re-entry. tracing::warn!(..., "deterministic projection rejection committed") still fires on every drain that hits a first refusal. If the host re-ticks this queue on the 60s scheduler, this is another Plan 26 WARN loop. Gate/backoff the repeat warn, or treat a standing durable skip like session_evidence_budget_exhausted. The isolated test only exercises after_deterministic_refusal and a hardcoded 1 < 8; it does not drain a multi-item batch.
producer.rs — cheaper-path, not a deadline bailout. Moving frontier init off worker-start onto the idle run_one_rollup_maintenance path (5 min ROLLUP_IDLE_RETRY_INTERVAL) and refusing persist-wake before init is less write-lock work. Same persistence deadline. Good. should_wake_rollup_now tests the gate; keep init on the idle arm (it still calls initialize_observability_rollup_frontier).
Stay stacked after #538. I will not flatten #543.
RetrievalBudget.deadline_micros is a request/profile bound, not publication or activation. new_admitted still hard-wires None for the daemon first-query caller.
Yield the rest of a projection drain after the first durable sanitization/contract skip so the batch does not keep paying receipt construction. Initialize the observability rollup frontier only on the idle tick, and do not persist-wake that write while it is uninitialized.
A standing SanitizationRefused skip is already cheaper (yield the rest of the batch). Re-warning on every 60s host tick is the Plan 26 storm.
The isolated multi-item yield proof left project_calls untyped, so saturating_add failed to compile in the lib test build.
3f26ecf to
ae9824b
Compare
|
Addressed the Plan 26 WARN backoff and restacked crate-only onto #538.
|
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Re-check on 3f26ecf34 (warn gate).
The gate is the right Plan 26 shape: skip + Deferred + yield still run every time; only the repeat WARN is held. Same observation id inside 3600s is Suppressed; a new id Emits. Isolated tests now cover the multi-item yield and the window. Not a deadline bailout.
Two leftovers, same as #538:
DeterministicRefusalWarnBackoff::new(0)—now < now+0is false, so a zero window always Emits. Rejectnew(0)(or treat it as invalid), don’t silently re-WARN every tick.- The 1h default is frozen (
DEFAULT_… = 3600+ processOnceLock). Fine for a warn-only gate; if this becomes a scheduler skip later, make the window Plan 20-overridable and give Suppressed its own typed skip (do not remap toscheduler_cooldown_active).
Crate-only on this commit. I am not editing or flattening.
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Deep review of current #546 tip ae9824bfe (after 3f26ecf34 / c805f2450). GitHub comments only.
Yield after first refusal is a silent drain-shape change. Skip + Deferred + break is cheaper per tick (1 project_observation instead of max). Confirm the store actually advances next_queued_observation past a durable SanitizationRefused/Contract skip. If the refused id stays at the head, every 60s tick pays one project and never reaches later healthy items — a stall, not less work. If it does advance, a refusal-heavy queue becomes one item per scheduler tick instead of a batch (cheaper/tick, slower calendar). Either way, say which one this is.
Process-global warn gate is untested on the live path. record_deterministic_refusal_warn uses a process OnceLock<Mutex<…>>. Tests only drive warn_gates_for_refusals with a local standing. The mutex/poison/id-overwrite path never runs. Two providers in one process share one anchor: a different observation_id Emits (ok), a reused id across scopes would suppress (silent).
Same leftovers as #538. DeterministicRefusalWarnBackoff::new(0) always Emits (now < now+0 is false). Reject it. Same-id after a sanitizer/content change stays suppressed for an hour — warn-only, skip still runs.
Slop. after_deterministic_refusal always returns (n+1, true, true) then debug_assert!(stop); break; — the bool is dead. The comment about if stop { break } typing as () is cargo-cult. The first test still asserts a hardcoded 1 < 8. ae9824bfe typing project_calls is fine.
Frontier idle-init still looks like the right cheaper path (not a deadline bailout). Not editing. Not remapping any base.
Prove a longer lane cannot lift a tighter base, and that Some(0) expires immediately instead of falling back to the crate default.
Yield after first refusal stalled later healthy queue items; store already consumed the refused id.
|
Store already consumes the refused id; drain now continues so later healthy items are not delayed; warn gate unchanged; no restack. |
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Re-check on 7f84b9b5a (continue after durable skip). Comments only. No restack.
This is the cheaper path, given consume is real. persist_projection_rejection_on_database already apply_skip_disposition + consume_projection_queue_item before the drain sees SanitizationRefused/Contract. Breaking after that was a stall of later healthy items until the next 60s tick, not less work. Continue + skipped += 1 + warn gate is one project per unique id. The 685-hit storm was re-entry, not "more than one refusal in a batch."
A refusal-heavy queue still pays sanitization/receipt once per remaining item this tick. That is one-shot, not the loop. Do not re-yield.
Leftover slop: test is still named first_deterministic_refusal_is_one_skip_and_yields_the_rest and now asserts the opposite. after_deterministic_rejection still returns two always-false bools. new(0) saturates to 3600 instead of making 0 unrepresentable (fine for warn-only; #538 used NonZeroU64). Process-global OnceLock still untested on the live path.
Frontier idle-init unchanged. Ready watch stays #538.
Thin wrapper so builder can pass a set request deadline_micros. None still uses the crate 30s fallback. new_admitted stays hard-wired None. No daemon edit.
…udget-backoff-9603' into codex/pr546-final
Live daemon mount is new_admitted_with_budget. new_admitted still hard-wires None.
The helper only added one to skipped. Keep draining after a durable store-consumed refusal; do not wrap that increment.
|
Leftover slop from the 7f84 re-check is gone: skip increment is inlined (warn gate already removed in 482286a). Marked ready so CI can run. Base unchanged. No merge. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b56552a0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut frontier_initialization_pending = true; | ||
| // Frontier write is idle-only. Starting it here raced persist for the | ||
| // same 2s write deadline (live: exceeded persistence deadline). | ||
| let rollup_tick = sleep_until(Instant::now() + ROLLUP_IDLE_RETRY_INTERVAL); |
There was a problem hiding this comment.
When a restart has pending durable outbox rows but receives no new observations, replacing the startup initialization arm with this timer removes the only startup recover_pending call. The timer performs only rollup maintenance, so those rows remain pending indefinitely—even replaying the owner fact returns Replayed without settling it—until unrelated telemetry or graceful shutdown triggers recovery. Keep an idle startup recovery trigger independent of frontier initialization.
AGENTS.md reference: AGENTS.md:L81-L83
Useful? React with 👍 / 👎.
| let mut frontier_initialization_pending = true; | ||
| // Frontier write is idle-only. Starting it here raced persist for the | ||
| // same 2s write deadline (live: exceeded persistence deadline). | ||
| let rollup_tick = sleep_until(Instant::now() + ROLLUP_IDLE_RETRY_INTERVAL); |
There was a problem hiding this comment.
For an idle producer with pre-existing dirty days or frontier work, this postpones the only maintenance attempt by the five-minute ROLLUP_IDLE_RETRY_INTERVAL. Consequently, observability_rollup_convergence.rs::idle_producer_converges_dirty_days_into_application_readable_fragments and observability_empty_day_closure.rs::idle_producer_publishes_one_proved_completed_quiet_day, which require convergence within three and two seconds respectively, now time out, and production rollups remain stale for the same interval. Schedule the first idle tick immediately while retaining the biased data/control priority.
Useful? React with 👍 / 👎.
| SimulatedProjectOutcome::Refusal, | ||
| SimulatedProjectOutcome::Projected, | ||
| ]; | ||
| let (skipped, project_calls) = simulate_drain_project_calls(&batch); |
There was a problem hiding this comment.
The helper invoked here never calls HostAdmissionFacade::drain_projection_queue and unconditionally visits every slice element, so this test remains green if the real durable-refusal arm regresses from continue to break or an early return. Exercise the facade/store path with a durable refusal followed by a healthy item so the asserted continuation behavior is actually falsifiable.
AGENTS.md reference: AGENTS.md:L78-L81
Useful? React with 👍 / 👎.
…er' into td/pr545-update
…udget-backoff-9603' into td/pr546-update
1 participant
Summary
SanitizationRefused/Contractskip, yield the rest of the batch (Deferred). Stops the 685-hit sanitization/receipt-construction hot loop. Plan 23/26 typed skip + Deferred — not a publication receipt.Test plan
cargo test -p tracedecay-usecases --lib first_deterministic_refusal_is_one_skip_and_yields_the_rest persist_wake_does_not_retry_uninitialized_frontier