fix(gateway): preserve exact history attribution beyond event page limits

[Beautyl0ve]

Fixes #4949

Why

GET /api/threads/{thread_id}/history previously inspected only the newest 1,000 run events when reconstructing an AI message's run. On long-lived threads, an older exact event could fall outside that window, causing the response to use a later turn's run ID and duration.

What changed

• Legacy checkpoint AI messages now use a complete-or-error, newest-first event lookup with a stable high-watermark. Unsafe/non-progressing cursors fail closed instead of producing false attribution.
• The write-on-read migration caches a complete run_message_ids audit (including exhaustive boundary misses) plus required run_durations, so subsequent reads avoid rescanning migrated IDs.
• After acquiring the durable checkpoint-write reservation, the migration re-queries every audited AI message ID and batch-reloads required run rows. A newer exact event or final duration therefore cannot be overwritten by a foreground snapshot.
• Long migrations size the initial run query to the required ID set and use targeted get() calls only for stragglers.
• Memory/database stores use bounded pagination; JSONL resolves from one validated, locked thread snapshot.
• Regression coverage includes attribution past 10,000 newer events, unsafe cursors, owner scoping, incremental cache coverage, both reservation races, and old-run hydration.

Concurrent upstream work

• fix(runtime): preserve per-run files in JSONL batch writes #4938 is merged and included in this branch; the combined JSONL behavior is covered by the focused event-store suite.
• fix(history): early user messages vanish or jump mid-run when pagination and context compaction overlap #4696 remains open and overlaps several files, but addresses checkpoint sequence positioning rather than exact run attribution. This branch is refreshed against current main; the overlap should be rechecked if fix(history): early user messages vanish or jump mid-run when pagination and context compaction overlap #4696 lands first.

Surface area

• Frontend UI — page / component / setting / interaction under frontend/
• Backend API — endpoint / SSE event / request-response shape under backend/app
• Agents / LangGraph — agent node, graph wiring, langgraph.json, or prompt change
• Sandbox — docker/ or sandboxed execution
• Skills — change under skills/
• Dependencies — new/upgraded entry in backend/pyproject.toml or frontend/package.json (say what it buys us)
• Default behavior change — changes existing behavior without the user opting in (default model, default setting, data shape)
• Docs / tests / CI only — no runtime behavior change

Screenshots / Recording

Not applicable; this is a backend history-attribution fix.

Bug fix verification

• Test paths: backend/tests/test_threads_router.py and backend/tests/test_run_event_store.py.
• No pristine-main run was performed. The regressions encode the old single-page failure and the two pre-admission snapshot races; the focused suite is green on this branch.

Validation

cd backend
pytest tests/test_run_event_store.py tests/test_threads_router.py tests/test_delta_channel_checkpointers.py tests/test_agent_guidance_check.py -q
182 passed, 4 skipped, 1 pre-existing Starlette deprecation warning

ruff check app/gateway/routers/threads.py packages/harness/deerflow/runtime/events/store/base.py packages/harness/deerflow/runtime/events/store/jsonl.py packages/harness/deerflow/runtime/runs/worker.py tests/test_run_event_store.py tests/test_threads_router.py
All checks passed

ruff format --check <same six Python files>
6 files already formatted

git diff origin/main --check
Passed

AI assistance

Tool(s) used: Codex

How you used it: Implementation, regression tests, documentation, and local validation.

• I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output.

[willem-bd]

Reviewed at e474610. The core design is sound: the complete-or-error pagination contract in RunEventStore.find_latest_ai_message_run_ids() is well-specified (stable high-watermark cursor, non-progression detection, exclusive before_seq reuse), the JSONL one-snapshot override is the right call given that backend's full-rescan pages, and the reservation-scoped revalidation of boundary fallbacks closes the flush/admission race cleanly. The test coverage is genuinely strong (10k-event pagination, owner scoping, unsafe-cursor abort, the reservation race). Findings below are all suggestion/nit level: three around read-amplification and response completeness on edge paths, plus one naming nit. Nothing blocking from my side.

[willem-bd]

Suggestion (read amplification): on the first migration read of a long thread, required_run_ids can contain one distinct run per AI message, and everything outside list_by_thread's default newest-100 page is hydrated with a sequential await run_mgr.get(...) — for a 300-turn unmigrated thread that is ~200 back-to-back store roundtrips inside a single synchronous history request. Since RunManager.list_by_thread already accepts limit (and even over-fetches to cover memory/store duplicates), sizing the initial call as limit=max(100, len(required_run_ids)) would collapse the common case to one query and reserve the per-id get() loop for the genuinely stragglers. It is one-time per thread thanks to the cache, but the first read after deploy is the one users will feel.

[willem-bd]

Suggestion: on a failed/incomplete exact lookup this re-raise skips stamp_turn_duration_on_last_ai entirely, so the response drops turn_duration for every message — including runs whose durations were already proven in checkpoint_run_durations and were never in question. The previous code stamped the checkpoint-known durations before touching the event store, so a fallback failure degraded only the unattributed turns. The PR description frames this as removing synthesized attribution it cannot prove, which the run_id pop already accomplishes; stripping provable durations is broader than that goal. Stamping with checkpoint_run_durations in the failure path (after the pop, when the only remaining run_ids are persisted-mapping ones) would keep the failure scoped to the messages that actually lost attribution.

[willem-bd]

Suggestion: the docstring states the mapping "must never contain data outside the checkpoint's materialized history", but nothing enforces that — the merge only ever adds entries. Two paths can violate it: (1) summarization/compaction removes messages from the messages channel, yet their ids stay in run_message_ids forever and are copied into every later checkpoint; (2) a body.before history read audits an older checkpoint whose messages may no longer exist in the head, persisting ids the head will never look up. Stale entries are harmless for correctness (reads look up only present ids) but they are dead weight baked into every subsequent checkpoint write. Consider pruning ids not present in the head checkpoint's messages at persist time, or softening the invariant sentence to "entries are retained permanently once written" so the documented contract matches the code.

[willem-bd]

Nit: importing _match_ai_message_run_id / _normalized_message_ids couples this backend to base internals — the leading underscore signals module-private, yet these are now the shared matching contract used by both the default implementation and the override. Dropping the underscore (or moving them next to IncompleteMessageRunLookupError as public helpers) would keep the cross-module dependency honest.

[Huixin615]

[P1] Only foreground misses in fallback_message_ids are revalidated after acquiring the checkpoint-write reservation. Exact hits and durations still come from the pre-admission snapshot.

This is unsafe because find_latest_ai_message_run_ids() deliberately freezes its view at the first page's high-watermark. A newer valid event for an already-resolved message ID can arrive while older pages are being scanned. Since that ID is absent from fallback_message_ids, the background task can persist the older run ID permanently, and subsequent history reads will trust run_message_ids without consulting the event store again.

The same race exists for duration data: a history request can compute a partial lifetime while the run is active, the run can then finish and persist its final duration, and this background task can overwrite it with the stale pre-admission value.

Please make the admitted phase authoritative: re-query every audited AI message ID and batch-hydrate/recompute the required run durations inside the reservation, then persist only those post-admission results. A regression test should cover both a newer exact event arriving after the foreground high-watermark and a final duration being written before the background metadata merge.

@Beautyl0ve Thanks for your PR, please take a look at the suggestion.

[Beautyl0ve]

@Huixin615 Addressed in d6180a1fc:

• the admitted phase now re-queries the complete audited AI-message ID set, so a post-admission exact mapping replaces either a foreground exact hit or boundary fallback;
• required run rows are batch-hydrated and durations recomputed inside the reservation before the metadata merge.

Added deterministic regressions for both the newer-exact-event race and the final-duration race. The focused suite is green (182 passed, 4 skipped), with latest main merged.

[willem-bd]

Latest-head follow-up: the event-attribution revalidation is now authoritative, but one blocking cross-worker duration race remains. Details are inline.

[willem-bd]

[P1] The checkpoint-write reservation does not make this duration snapshot authoritative. Durable admission excludes only pending/running rows, while run_agent terminalizes the row before update_run_completion() advances updated_at and before _persist_run_duration() writes the final checkpoint duration. On another Gateway worker, this task can therefore acquire the reservation, read stale timestamps, and then race the terminal worker's remaining checkpoint write; persist_run_history_metadata() can overwrite the newer duration, after which history trusts the stale value permanently. I reproduced the merge failure by writing a final 9-second checkpoint duration while this migration held its reservation: the migration replaced it with its 3-second snapshot. Please keep the run active through the final duration write, or use cross-worker ordering/merge semantics that cannot overwrite a newer duration, and add a regression matching the real finalization order.

[Beautyl0ve]

Addressed in d277908. The worker now keeps the durable run row active through the final duration checkpoint write: it persists the final completion fields/timestamp as progress while the row is still running, writes _persist_run_duration(), and only then persists the terminal status. A peer checkpoint_write reservation therefore cannot enter during that window.

I also added a cross-worker regression matching the finalization order: a second RunManager attempts the reservation from inside the duration write and is rejected while the durable row is still running; the row becomes success only afterward. Validation: 186 focused history/worker tests plus 60 run-manager tests passed; Ruff, diff, and agent-guidance checks passed.