Skip to content

fix(nap): archive undated decisions and add count-based fallback#38

Closed
diberry wants to merge 1 commit intodevfrom
squad/20-fix-nap-archival-bugs
Closed

fix(nap): archive undated decisions and add count-based fallback#38
diberry wants to merge 1 commit intodevfrom
squad/20-fix-nap-archival-bugs

Conversation

@diberry
Copy link
Copy Markdown
Owner

@diberry diberry commented Mar 26, 2026

fix(nap): Archive undated decisions and add count-based fallback

Fixes #20decisions.md grows unbounded because archiveDecisions() silently fails on real-world content.

Bugs Fixed

Bug 1: Undated entries immune to archival

  • daysAgoFromLine() returns null for entries without YYYY-MM-DD dates
  • The split logic put null daysAgo into recent → never archived
  • Fix: Three-bucket split (old/undated/recent). Undated entries are archived alongside aged-out ones when file exceeds threshold.

Bug 2: Age-only pruning fails for active projects

  • If all entries are <30 days old, old.length === 0 → returns null regardless of file size
  • Active projects blow past 20KB in days with all entries recent
  • Fix: Count-based fallback. When nothing is age-archived, sorts entries oldest-first and greedily keeps the most-recent entries that fit under the 20KB threshold.

Evidence

Both bugs confirmed on bradygaster/squad@dev — verified the code is identical upstream. This repo's decisions.md is at 345KB (17× the 20KB threshold). Archival has been silently returning null since Feb 28.

TDD Approach

5 new test cases written first (red), then implementation (green):

  1. archives undated entries when file exceeds threshold
  2. archives mixed dated+undated — keeps recent dated, archives undated
  3. archives entries when all are recent but file exceeds threshold (count-based fallback)
  4. preserves all entries — no data loss during archival
  5. secondary pass keeps most recent entries by date, archives oldest

Result: 43/43 nap tests pass. No existing tests broken.

Changes

File Lines What
packages/squad-cli/src/cli/core/nap.ts +43 3-bucket split + count-based fallback in archiveDecisions()
test/nap.test.ts +156 5 new test cases + generateDecisions() helper

Postmortem

Full postmortem posted on #20.

Root cause: Commit aaefeb1 (Feb 28) — Copilot wrote implementation + 38 tests in one pass. Tests used synthetic 2024-01-XX dated fixtures, never challenging the "entries always have dates" assumption. The happy path worked; adversarial cases were never considered.

Key lesson: "AI writes the tests it believes in, not the tests that break it." Adversarial test generation must be a deliberate separate step by a different agent.

10-Agent Team Review

Issues #20 and #21 received full team reviews from Flight, EECOM, FIDO, Procedures, and Telemetry. Key findings:

  • squad doctor already exists (extend, don't recreate)
  • Zero squad.decisions.* OTel metrics exist (subsystem is completely dark)
  • Union merge driver (merge=union) conflicts with line-deletion archival
  • Highest-ROI future fix: coordinator pre-spawn decision filtering (API already supports it)

Follow-up Issues Created

# Title Owner
#22 ## heading awareness in archiveDecisions EECOM
#23 Data-loss invariant test FIDO
#24 Scribe charter format mandate Procedures
#25 Silent-failure detection in nap report EECOM
#26 Archival test checklist for CONTRIBUTING Procedures
#27 Coordinator pre-spawn decision filtering EECOM
#28 decisions-index.md as separate file EECOM
#29 squad.decisions.* OTel metrics Telemetry
#30 Extend squad doctor with size checks EECOM
#31 Troubleshooting guide docs Procedures
#32 Coordinator diagnostic breadcrumbs as OTel Telemetry
#33 Scribe task reorder with HARD GATE Procedures
#34 Configurable thresholds in config.json EECOM
#35 Decision lifecycle states EECOM
#36 Ralph idle-watch archival trigger EECOM
#37 Contract comment for archiveDecisions EECOM

Bug 1: Undated entries (### headings without YYYY-MM-DD) were immune to
archival because daysAgoFromLine() returns null and the split logic
treated null as 'recent'. Now treats undated entries as archivable when
file exceeds threshold — they are archived before dated recent entries.

Bug 2: When all entries are <30 days old, archiveDecisions() returned
null regardless of file size. Added count-based secondary pass that
keeps the most recent entries and archives the rest when the file
exceeds threshold after age-based pruning.

Fixes #20

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 26, 2026

Superseded by bradygaster/squad PR bradygaster#627 which merged the count-based fallback with Procedures' correction (undated entries preserved). Our fix in this PR was partially rejected by Procedures for the same reason Brady adopted. No longer needed.

@diberry diberry closed this Mar 26, 2026
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.

2 participants