fix(ledger): report the history a bounded write discards

[chenmingwei23]

What is the problem?

session_ledger._serialize_bounded() keeps the state document under the ceiling its own reader enforces by evicting the oldest events, then the oldest tried entries, then unknown forward-compat fields wholesale. It does all of that silently - there is no log line anywhere on the write path.

The issue that prompted this reports a different defect: that record() returns a dict the serializer mutated, so the caller sees fewer entries than are on disk. That claim is inverted and I did not implement its proposed fix. The eviction loop re-serializes after every eviction and returns the blob built from the fully-evicted dict, so the returned dict already equals the bytes written. Applying the proposed _serialize_bounded(dict(state)) would create the divergence it claims to remove, returning the pre-eviction lists while disk held the evicted ones. Mutation-verified below.

The genuine residual is the silence, and the missing test coverage underneath it. Two coverage facts I measured rather than assumed:

• test_writer_guarantees_the_read_ceiling_for_legitimate_records never reaches the eviction branch. A full astral-script event tail is ~806 KB, under the 995,904-byte budget on its own; crossing it needs events and tried both at maxima (~1.87 MB). Running that test with the new warning in place emits zero lines, confirming it.
• So on main the events/tried eviction branch has no coverage at all. Only the extras-drop branch does, via test_oversized_unknown_fields_are_dropped_not_self_corrupting.

Why this issue matters to the user

A read-side clamp is recoverable: the original file is still on disk until something writes back. A write-side eviction is not. The dropped entries never reach disk, so once the write lands the loss is permanent, and nothing tells the operator it happened. A long-horizon session whose tried/events history is being quietly truncated by the write budget looks identical to one that simply has less history, and the ledger exists precisely so that history survives compaction.

This is also an asymmetry on main today, independent of any other PR: _read_state_unlocked() already logs WARNING "ledger state file over size ceiling; treating as absent" when that same ceiling makes it discard a whole file. The writer discarding history to stay under it says nothing.

How our fix solves it

Symptom: a session's ledger history shrinks with no explanation. Cause: _serialize_bounded() evicts to satisfy _MAX_STATE_BYTES and reports nothing. Fix: tally what each eviction drops and emit one line per over-budget serialization.

• _serialize_bounded() counts evicted events, evicted tried, and dropped unknown keys, then logs a single WARNING naming the budget, the ledger directory, and each loss with its count. A document that fits logs nothing, so this is not per-write noise.
• The tally is emitted from a finally, so the ValueError("record too large to store") refusal path reports too. That path raises with the in-memory record already stripped of its history, which is the moment the operator most needs to know what the call threw away.
• The line describes the document the call built, not a durable write. atomic_write runs after the serializer and can fail (ENOSPC), leaving the previous state file whole; the refusal path never writes at all. Claiming a write discarded data would be false in both cases. This wording was tightened in response to GPT's advisory finding on the first revision, and is now pinned by a test.
• record() passes source=dir_path.name so the line names which ledger lost data, mirroring the read-side _coerce_state(raw, source=...) shape in open PR fix(ledger): report the data a state read discards instead of losing it silently #6017.
• Zero behaviour change: what is kept, what is written, and what is returned are all byte-identical to before. This is observability only.
• The aliasing the issue asked to remove is instead documented and pinned: a comment at record()'s return states why a copy must not be serialized there, and a test enforces the equality.

What tests we did

test/test_session_ledger.py, targeted file only - 52 passed. black, isort, flake8, mypy clean on both changed files.

New: TestBoundedWriteIsLoudAboutLoss (eviction counted and ledger named; unknown fields named; refusal path still reports; a failed write still reports while leaving the stored file intact; a record that fits logs nothing; plus the budget arithmetic documenting why the fast tests squeeze the ceiling instead of using production bounds) and test_record_returns_exactly_what_landed_on_disk.

Verified red, not just green. Three mutations, each isolating one property:

Run	Result
Fix applied (baseline)	7 passed
Source reverted to origin/main, tests kept	3 failed, 3 passed
Mutation A - logger.warning suppressed, signature kept	the 4 loud tests fail, nothing else
Mutation B - the issue's proposed _serialize_bounded(dict(state))	only test_record_returns_exactly_what_landed_on_disk fails
Mutation C - message reworded back to claim a completed write	only test_a_failed_write_still_reports_but_leaves_the_stored_file_intact fails

Mutation A matters because the base-revert reds fail on the added source parameter, which would prove nothing about the warning; suppressing only the log call reddens the same tests, so the loud assertions are load-bearing. Mutation B is the evidence for rejecting the issue's proposed fix. Mutation C proves the ENOSPC test pins the wording rather than merely passing alongside it.

The eviction tests squeeze _MAX_STATE_BYTES via monkeypatch rather than building a 1.87 MB document. test_record_returns_exactly_what_landed_on_disk and the ENOSPC test both derive their squeezed ceiling from the real file size, so a timestamp-width change cannot silently turn either into a no-op.

Any other suggestions on the work?

1. The issue should not be closed as not-a-bug, but its Proposed fix section should be struck. A comment on session_ledger: record() returns a dict _serialize_bounded mutated in place (write-side sibling of #6017) #6290 records the disposition; this PR keeps the correct behaviour and pins it so the shallow copy cannot be reintroduced as a cleanup.
2. The eviction loop is O(n) re-serializations of the whole document under the exclusive lock - up to 150 iterations (_MAX_EVENTS + _MAX_TRIED), each a full json.dumps of an up-to-1.8 MB dict, each dropping exactly one entry. With _LOCK_TIMEOUT_SECS = 5.0, a slow eviction can push a concurrent writer past its deadline into a refused write (OSError, surfaced as 503 by the dashboard handler). Deliberately out of scope here; a size-guided bulk trim would collapse it to a couple of passes. Happy to file it separately.
3. Overlap with fix(ledger): report the data a state read discards instead of losing it silently #6017 is by construction, not accident. That PR makes the read side loud; this one makes the write side loud, in a different function (_serialize_bounded vs _coerce_state). If fix(ledger): report the data a state read discards instead of losing it silently #6017 lands first, the rebase should fold this PR's inline summary rendering into its _lost_summary() helper - I kept the rendering local rather than pre-adopting a helper that does not exist on main.
4. _serialize_bounded() evaluates _empty_state() once per key inside the extras comprehension. Untouched to keep this diff observability-only.

Pattern harvest

Rule candidate: review-prompt
Pattern: a bound-enforcing path discards caller data with no diagnostic - and on a write path that discard is unrecoverable, because the dropped bytes never reach disk for anything to read back.

This is the second instance in this one module: open PR #6017 fixes the read-side clamps and this fixes the write-side evictions, in a file whose own reader already warned about the whole-file case. The generalizable review question is not "is the bound correct" but "when this bound sheds data, can an operator find out - and is there still a copy anywhere". A semgrep rule is not the right vehicle: the shape (a re-serialize-and-trim loop with no logging in the enclosing function) is too fuzzy to match without heavy false positives, whereas a reviewer prompted to ask it will catch the whole class.

Closes #6290

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 83dd9f5014e01f6f6e0912196425324a42017ee3 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Silent, unrecoverable write-side eviction now reports at the layer that does the evicting, with the return-value/disk aliasing pinned as contract — sound and proportionate.

[DESIGN-REVIEWED] 83dd9f5

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 83dd9f5014e01f6f6e0912196425324a42017ee3 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 83dd9f5

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 83dd9f5014e01f6f6e0912196425324a42017ee3: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 83dd9f5014e01f6f6e0912196425324a42017ee3 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 83dd9f5

Verdict parsed from the review's SHA-scoped output markers for commit 83dd9f5014e01f6f6e0912196425324a42017ee3.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 83dd9f5014e01f6f6e0912196425324a42017ee3: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 83dd9f5014e01f6f6e0912196425324a42017ee3 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Both data files read, the changed module read in full, and consumer counts run. Composing the review now.

First-Principles-Verdict: PASS

One reported, unrecoverable discard site made loud at the exact function that discards; every rider is a pin against a filed issue's inverted fix.

What this change ships

Intent: let an operator find out when the ledger's write budget permanently throws away session history — a FIX for the silence, explicitly not for issue #6290's (inverted) claim.

1. Over-budget ledger writes now log one WARNING counting evicted events/tried and dropped unknown fields — justified (mirrors the existing read-side warning at session_ledger.py:272; the write-side loss is unrecoverable).
2. The "record too large to store" refusal also reports what it gutted — justified (same harm, worst moment).
3. The warning names which ledger lost data via a new source param — one consumer, generalized (default "" unused by its single production caller, session_ledger.py:384).
4. record()'s return-the-mutated-dict aliasing is now commented and pinned by a test — declared; blocks session_ledger: record() returns a dict _serialize_bounded mutated in place (write-side sibling of #6017) #6290's proposed copy from creating real divergence.
5. Fitting documents stay silent — justified control against per-write noise, pinned.
6. Spec section added to session-work-ledger.md — mandated by the same-commit spec rule.
7. Constants-arithmetic test documenting why eviction tests squeeze the ceiling — declared; keeps the fast tests from going no-op.

Grepped _serialize_bounded across the repo: 1 production caller; no existing mechanism reports this discard (2 logger.warning sites in the module, the other covers only whole-file read discard). The record() ring-buffer caps (events[-_MAX_EVENTS:], tried[-_MAX_TRIED:]) are not unfixed siblings — bounded aging is the module's documented contract, and warning there would be the per-write noise item 5 exists to prevent.

Subtractions

• Drop the source: str = "" default and the f" ({source})" if source else "" branch in _serialize_bounded — the only production caller (session_ledger.py:384) always passes one; take a required parameter.

[FIRST-PRINCIPLES-REVIEWED] 83dd9f5

[chenmingwei23]

Pushed 83dd9f501. Dispositioning the round-1 review output.

GPT advisory (session_ledger.py:467, ENOSPC makes the message overclaim): ACCEPTED and fixed. It is right. The warning fires inside _serialize_bounded, which runs before atomic_write, and on the refusal path no write happens at all - so "ledger write discarded data" asserted a durable loss that in both cases had not occurred. The message is now "ledger state exceeded the <N>-byte serialization budget (<ledger>); discarded: ...", which describes the document the call built. The in-memory record really is stripped either way, which is why the line still fires; only the durability claim was wrong.

Rather than just reword, I pinned it: test_a_failed_write_still_reports_but_leaves_the_stored_file_intact patches atomic_write to raise OSError(ENOSPC), asserts the discard is still reported, asserts the stored file is byte-identical afterwards, and asserts the message contains no "write" claim. Mutation C in the PR body reverts only the wording and reddens exactly that test and nothing else.

PR Hygiene (red): fixed. The body was missing the ## Pattern harvest section required on fix PRs. Added, with Rule candidate: review-prompt - this is the second instance of the same class in this one module (#6017 is the read-side sibling), and the generalizable question is whether a bound that sheds data leaves an operator any way to find out.

Design Review, Opus and First Principles were PASS with no findings on the previous head; all three re-roll on this push.

[bolichen97]

Tier 1 auto-approve: fix (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix with a clear root cause — reports (WARN) the history a bounded ledger serialization discards, and returns the evicted dict so the caller's post-write view matches disk (#6290). Spec files changed as a ride-along (a minority of the diff on both file count and changed lines), not reviewed as a design decision: docs/system-specs/features/session-work-ledger.md.

[bolichen97]

Tier 1 auto-approve: fix (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: bounded ledger serialization discarded oldest events/tried entries with no log line, so an unrecoverable partial write was silent; the fix adds one WARNING per over-budget serialization naming the ledger and the evicted counts, and pins that record() returns the same dict the eviction mutated so the caller's post-write view equals disk. Spec files changed as a ride-along (a minority of the diff on both file count and changed lines), not reviewed as a design decision: docs/system-specs/features/session-work-ledger.md.

[iamwhatever]

Tier 1 auto-approve: fix (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: the ledger's serialization budget silently evicted the oldest events[]/tried[] entries with no operator signal — unlike the read side there is no original file left to recover them from, so this adds one warning per over-budget serialization naming what went and how much. Spec files changed as a ride-along (a minority of the diff on both file count and changed lines), not reviewed as a design decision: docs/system-specs/features/session-work-ledger.md.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #6017 is PARTIALLY_COVERED relative to this PR. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #6017: REBASE. PR #7631 implements the write-side half of the same goal and is already merged; it neither implements nor obsoletes the read-side coercion caps this PR reports. Its only effect on this PR is the test-file adjacency conflict it created. Files: src/kiro_crew/session_ledger.py, test/test_session_ledger.py, docs/system-specs/features/session-work-ledger.md.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.