Skip to content

fix(anatomy): never delete entries a capped scan did not reach - #70

Open
liveoakwag wants to merge 1 commit into
cytostack:mainfrom
liveoakwag:fix/anatomy-never-shrink-on-capped-scan
Open

fix(anatomy): never delete entries a capped scan did not reach#70
liveoakwag wants to merge 1 commit into
cytostack:mainfrom
liveoakwag:fix/anatomy-never-shrink-on-capped-scan

Conversation

@liveoakwag

Copy link
Copy Markdown

walkDir returns the moment the collected set reaches anatomy.max_files, and it walks each directory in alphabetical order. So a capped run does not sample the tree — it stops partway through it. scanProject then did existing.files = fresh.files, which reads "I never got there" as "this file is gone" and deletes the entire alphabetical tail.

On one real project — 4,969 eligible files, 530 indexed, cap 500 — a scan kept 245 files from an early bulk directory and would have removed every entry under Strategies/ (1,546 files), 05 Decisions/, 07 Dashboards/ and 09 Coordinator/. The entry count only fell 530 → 500, so nothing looked wrong afterwards. That is what makes it nasty: the damage is invisible in the summary line.

It is also reachable with nobody typing openwolf scansrc/daemon/cron-engine.ts runs scan_project on a schedule.

The change

A truncated walk has no way to tell the two cases apart, so it is no longer allowed to delete: it merges and prints what it kept. Pruning stays available on any complete walk, which is every project under its cap — so behaviour is unchanged for anyone the bug never affected.

  • buildAnatomy now returns truncated. It is fileCount >= maxFiles, not >, because a walk that ended exactly at the cap is indistinguishable from one that would have continued, and the safe reading of an ambiguous signal is the pessimistic one.
  • The lock-contention fallback is skipped when truncated, for the same reason: it writes renderStore(fresh) straight to anatomy.md, and fresh holds only the part of the tree the walk reached. Losing the lock is not a reason to publish a partial render over the file humans read — and that importFromMarkdown reads back.

Verification

  • node --test tests/anatomy-scan-never-shrink.test.ts6/6
  • npm test32/32, 0 fail
  • npx tsc --noEmit — clean apart from the pre-existing src/daemon/cron-engine.ts(52,27) TS2503, present on an unmodified checkout too
  • git am onto pristine main (f64e737) applies cleanly, suite still 32/32

The test is shown to catch the defect rather than merely pass: reverting only the scanner change and rebuilding makes 5 of its 6 cases fail. The sixth is the under-cap case, which was never broken. It also covers the two things the guard must not cost you — a complete walk still prunes a genuinely deleted file, and curated md-import descriptions survive.

One thing I would like your view on

The test imports the built module and builds on demand if dist/ is absent. npm test runs off the TypeScript with type-stripping, which does not rewrite relative import specifiers, and anatomy-scanner.ts imports ./description-extractor.js. Every existing test happens to target a module with no relative imports, so this has not come up before. Happy to restructure — build the suite first, or split the scanner so the decision is unit-testable straight off the source — whichever you prefer.

Independent of #66 and #69; cut from main and does not depend on either.

🤖 Generated with Claude Code

`walkDir` returns the moment the collected set reaches `anatomy.max_files`, and it
walks each directory in alphabetical order. So a capped run does not sample the
tree - it stops partway through it. `scanProject` then did
`existing.files = fresh.files`, which reads "I never got there" as "this file is
gone" and deletes the entire alphabetical tail.

On one real project - 4,969 eligible files, 530 indexed, cap 500 - a scan kept 245
files from an early bulk directory and would have removed every entry under
`Strategies/` (1,546 files), `05 Decisions/`, `07 Dashboards/` and
`09 Coordinator/`. The entry count only fell 530 -> 500, so nothing looked wrong
afterwards. It is also reachable with nobody typing `openwolf scan`: the daemon's
cron engine runs `scan_project` on a schedule.

A truncated walk has no way to distinguish the two cases, so it is no longer
allowed to delete: it merges and prints what it kept. Pruning stays available on
any complete walk, which is every project under its cap - so the full-replace
behaviour is unchanged for anyone the bug never affected.

`buildAnatomy` now returns `truncated`. It is `fileCount >= maxFiles`, not `>`,
because a walk that ended exactly at the cap is indistinguishable from one that
would have continued, and the safe reading of an ambiguous signal is the
pessimistic one.

The lock-contention fallback is skipped when truncated, for the same reason: it
writes `renderStore(fresh)` straight to anatomy.md, and `fresh` holds only the
part of the tree the walk reached. Losing the lock is not a reason to publish a
partial render over the file humans read and `importFromMarkdown` reads back.

tests/anatomy-scan-never-shrink.test.ts covers it, including that a complete walk
still prunes a genuinely deleted file and that curated `md-import` descriptions
survive. Reverting only the scanner change and rebuilding makes 5 of its 6 cases
fail; the one that still passes is the under-cap case, which was never broken.

The test imports the built module and builds on demand if `dist/` is absent:
`npm test` runs off the TypeScript with type-stripping, which does not rewrite
relative import specifiers, and `anatomy-scanner.ts` imports
`./description-extractor.js`. Every existing test happens to target a module with
no relative imports, so this has not come up before - happy to restructure if you
would prefer the suite built first, or the scanner split so the decision is
unit-testable off the source.

  node --test tests/anatomy-scan-never-shrink.test.ts   6/6
  npm test                                             32/32
  npx tsc --noEmit                                     clean except the
                                                       pre-existing
                                                       src/daemon/cron-engine.ts
                                                       TS2503, present on an
                                                       unmodified checkout too

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant