Skip to content

perf(storage): prune debug retention from a single artifact scan - #729

Merged
Juliusolsson05 merged 3 commits into
mainfrom
perf/debug-retention-single-scan
Sep 3, 2026
Merged

Juliusolsson05 merged 3 commits into
mainfrom
perf/debug-retention-single-scan

Conversation

@Juliusolsson05

Copy link
Copy Markdown
Owner

Problem

pruneDebugStorage (src/main/storage/debugRetention.ts) runs every five minutes for the life of the process — performanceLog.ts schedules it on every perf flush. Each run called collectArtifacts() three times, once per pass (TTL, per-bucket cap, global budget). Each call re-reads and re-parses the debug-bundle ledger (18.4 MB / 45,428 lines on this machine → ~136k JSON.parse calls and ~55 MB of transient strings per prune) and stat-walks every tracked file (~6,500 stat round-trips per prune), all on the main thread. Details in #728.

What changed

  • One collectArtifacts() per prune. The three passes now run over that single list through a new exported runPrunePasses(artifacts, policy, remove) that keeps an in-memory live set: an artifact removed by an earlier pass is dropped from the set and never re-offered.
  • Byte accounting subtracts what the remover reports, which is exactly what the old code did between re-scans, so pass order, per-pass rules (protected artifacts, the manual bundle bucket, the active grace) and outcomes are unchanged. A removal that frees nothing stays in the working set, as a re-scan would have found it.
  • pruneDebugStorage is now policy resolution + one collect + the passes. DebugStorageArtifact, DebugStoragePrunePolicy and the passes result are exported so the pass logic can be tested without disk.

Linked issues

Fixes #728. Refs #103, #372.

Verification

  • NODE_ENV=test npx vitest run --project unit src/main/storage/debugRetention.test.ts — 8 tests passed: the three existing collectSessionRecordingDirs contracts plus five new in-memory runPrunePasses contracts (TTL removes only stale unprotected artifacts; cap pass trims oldest inactive artifacts of an over-cap bucket only; budget pass trims oldest-first across buckets; an artifact removed by one pass is never offered to a later one; a removal that freed nothing is kept in the working set and not counted).
  • npx tsc -p tsconfig.node.json --noEmit — clean.

Not in scope

The proxy bucket exceeding its cap (8.4 GB vs 3.86 GB): live Codex runs sit inside ACTIVE_GRACE_MS and single runs reach multiple GB because the Responses proxy mirrors every SSE chunk. That needs a per-run capture cap in codex-headless; tracked separately.

Plan: docs/superpowers/plans/2026-09-03-debug-retention-single-scan.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kk16MNVJtWAnCeGxGRuHqa

Juliusolsson05 and others added 3 commits September 3, 2026 12:47
Refs #728

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kk16MNVJtWAnCeGxGRuHqa
pruneDebugStorage runs every five minutes for the life of the process and
used to call collectArtifacts() once per pass — three parses of the
multi-MB debug-bundle ledger and three stat walks over every tracked file
per prune, on the main thread.

The passes now run over one collected list with an in-memory live set;
byte accounting subtracts what removeArtifact reports, exactly as the old
code did between re-scans, so pass outcomes are unchanged. The pass logic
is exported with the remover injected so it is unit-testable in memory.

Fixes #728
Refs #103

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kk16MNVJtWAnCeGxGRuHqa
…can trade-off

Review follow-ups: the remover now answers whether the artifact is gone and
the passes charge the artifact's collected bytes, so accounting cannot
drift between passes; the comment states what a single scan trades away
(bytes/mtime snapshot, files created or removed mid-prune) and why that is
acceptable for the seconds a prune takes; the cap and budget tests are
built so the protected and active artifacts are actually reached while
still over cap/budget; the artifact type is exported directly.

Refs #728

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kk16MNVJtWAnCeGxGRuHqa
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Self-review pass (adversarial review of the first cut) — resolved in HEAD:

  • Staleness under-documented (S1/N1) → the pruneDebugStorage comment now states what one scan trades away (bytes/mtime snapshot, files created or removed mid-prune) and why that is acceptable; now taken after the scan is deliberate and explained.
  • Remover contract (N2) → remove returns a boolean; passes charge the artifact's collected bytes, so pass 2 and pass 3 accounting cannot drift. Production wraps removeArtifact.
  • Cap test could not fail without the grace check (S2) → proxy cap lowered so protected and active artifacts are reached while still over cap; a manual-bucket artifact added; budget test gains protected/active cases (S3), plan doc claims aligned.
  • Type alias (N5) → DebugStorageArtifact exported directly.

Out of scope, filed separately: ghost logs are protected in every pass so the "cap handles abandoned ghost files" comment is currently false.

@Juliusolsson05
Juliusolsson05 merged commit d042319 into main Sep 3, 2026
2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the perf/debug-retention-single-scan branch September 3, 2026 20:09
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.

perf(storage): debug retention prune scans every artifact and parses the bundle ledger three times per run

1 participant