[fix] Last-message-only reconstruction: fix ordering, config wiring, and robustness - #5500
[fix] Last-message-only reconstruction: fix ordering, config wiring, and robustness#5500ardaerzin wants to merge 5 commits into
Conversation
…gerprinting Reconstruction ran inside runTurn, but two things that read request.messages already ran outside it: the turn's own prompt was persisted first, and the keep-alive history fingerprint was computed on the inbound (minimal) history. So on a fresh turn the record log already held the current prompt (reconstruction returned it, then the inbound tail was appended -> the prompt appeared twice), and every warm continuation mismatched the fingerprint the previous park predicted, evicting to cold. Hoist reconstruction to the session-owned run handler, before the prompt persist and before the fingerprint. At that point the log holds only prior turns (rebuilt history + inbound tail is the full conversation exactly once), and the fingerprint sees the same full history a full-history client would send, so warm continuations match. plan is then built from the full request, so runTurn no longer recomputes the transcript. Also tighten the reconstruct guard to exactly one trailing user message (empty / assistant-only inbound must not rebuild).
fetchSessionRecords sits on the turn's critical path (the prompt waits on it), so add an AbortSignal.timeout (env-tunable) to fail fast to the inbound-history fallback instead of hanging on Undici's long request-level timeout. takePersistFailures had no caller: the per-session drop count was written in durable mode and never read, so the signal was lost and the map grew unread. Consume it at the turn-end drain (flush) — warn when the durable log is incomplete and clear the counter. Add guard regressions for empty / assistant-only inbound and a flush-consumer test.
…esolves The publish path reads env.agenta.sessions.records.smart_truncation, but SessionsConfig was defined and never wired onto AgentaConfig. With AGENTA_RECORDS_SMART_TRUNCATION on, that read raised AttributeError inside the publish try/except, which logged and returned False — so an over-64KB record was dropped entirely instead of structure-truncated, losing conversation context now that records are the source of truth. Wire the sub-namespace and add a regression guard for the config path.
The runner service has no env_file by design, so AGENTA_SESSIONS_RECONSTRUCT / AGENTA_RECORDS_DURABLE / AGENTA_RECORDS_INGEST_MAX_RETRIES never reached it and the feature could only be toggled in unit tests. Forward them explicitly in the oss/ee dev compose runner env; all default OFF (empty -> disabled), so nothing changes unless set.
dosubot
Bot
added
the
size:M
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…uction seam Reconstruction from records supplies model context only in the cold-evicted case; warm and cold-not-evicted turns resume natively. On warm turns reconstruction runs only to realign the keep-alive fingerprint. Also record that the seam moved to the run handler (before persist and fingerprint).
dosubot
Bot
added
size:L
Railway Preview Environment
Updated at 2026-07-24T23:59:22.774Z |
|
Closing as a duplicate. I built this independently before seeing Mahmoud's QA notes — the same fixes are already up as focused PRs: #5488 (compose flags), #5489 (double prompt), #5490 (warm eviction), #5491 (SessionsConfig). Those are the canonical ones; this one restructured the reconstruction seam more broadly and overlaps them. The only bits not covered there are minor (a records-query fetch timeout and consuming takePersistFailures at turn end) — leaving those for a separate call rather than piling on. |
1 participant
Context
Stacked on #5486. The differential QA on that PR (comment) found four defects that a pass/fail gate can't see, plus CodeRabbit flagged the same config bug and a couple of robustness gaps. This PR fixes them. All four feature flags stay off by default, so nothing changes unless they're set.
What was broken and how it's fixed
1 + 2. Reconstruction ran in the wrong place (one root cause, two symptoms).
Reconstruction happened inside
runTurn, but two things that readrequest.messagesalready ran outside it: the turn's own prompt was persisted first (server.ts), and the keep-alive history fingerprint was computed on the inbound (minimal) history.[reconstruct] records=1 priorMessages=1 inbound=1). This fired even in full-history mode, because turn one always carries exactly one message.Fix: hoist reconstruction to the session-owned run handler, before the prompt persist and before the fingerprint. At that point the record log holds only prior turns, so rebuilt-history + inbound-tail is the full conversation exactly once, and the fingerprint sees the same full history a full-history client would have sent (so warm continuations match).
planis then built from the full request, sorunTurnno longer recomputes the transcript. The guard is also tightened to exactly one trailing user message, so an empty or assistant-only inbound (an approval resume) never rebuilds.Before:
messages: [{user: "PONG"}, {user: "PONG"}]on turn one; warm sessions evicted to cold every turn.After: turn one sends one message; warm continuations stay warm.
3. Smart truncation dropped the record instead of truncating it.
SessionsConfigwas defined inenv.pybut never attached toAgentaConfig, soenv.agenta.sessions.records.smart_truncationraisedAttributeErrorinside the publishtry/except, which logged and returnedFalse. WithAGENTA_RECORDS_SMART_TRUNCATIONon, an over-64KB record was dropped entirely — worse than the legacy path it replaced. Now records are the only copy of the conversation, so that's lost context. Fix: wire the sub-namespace ontoAgentaConfigand add a regression guard for the config path.4. The dropped-record signal was never consumed.
takePersistFailureshad no caller: in durable mode a dropped record was counted into a per-session map that nothing read, so the signal was lost and the map grew unread. Fix: consume it at the turn-end drain (flush) — warn when the durable log is incomplete and clear the counter.Records fetch could hang the turn (CodeRabbit).
fetchSessionRecordssits on the turn's critical path (the prompt waits on it). Added anAbortSignal.timeout(env-tunable, 5s default) so a stalled query fails fast to the inbound-history fallback instead of waiting on Undici's long request-level timeout.Flags couldn't reach the runner.
The runner service has no
env_fileby design, so the session flags never reached it and the feature could only be toggled in unit tests. Forwarded them explicitly in the oss/ee dev compose runner env; all default OFF.Tests
env.agenta.sessions.records.smart_truncationresolves.What to QA
Needs all four flags on. Turn 1 "my name is Arda", turn 2 "what's my name?".
/invokebody carries a single message and the runner logs[reconstruct] … inbound=1with no duplicated prompt (turn 1 should not reconstruct at all — records are empty before the prompt is persisted).[keepalive] mismatch (history) … evict + coldlines; turn 2+ latency stays at warm levels.Note
The one QA finding not addressed here is the FE gating (
agentRequest.ts): last-message-only should gate on a server-confirmed durable-history state rather than a non-empty session id, so a session with no records retains full history. That's a larger FE/BE contract change and belongs in its own PR.