Conversation
|
The Windows gate is red on this branch, and from the run's own JSON report the two files it flags as new are timeouts on a slow runner rather than anything this diff reaches: Both are timeouts, not assertion failures, and neither file imports anything this branch changes: the diff adds one constant to The same run shows the runner was slow throughout. I have left |
`Project fingerprint byte budget exceeded (1073741824)` named no path, so the one question it raised, which files did this, was the one it did not answer. Users were left scanning the filesystem by hand to find the weight before they could write an excludePatterns entry. The walk has already stat'd every admitted file by the time the read loop trips the budget, so the sizes needed to answer that are in memory and were being discarded. The error now reports the largest non-overlapping paths in the corpus, the corpus total against the cap, and what to do next. Non-overlapping is what makes the list usable: every file's size is rolled into each of its ancestor directories, then the totals are read from largest down, keeping an entry only when no ancestor or descendant of it has been kept already. Each line is therefore its own bytes and can go straight into excludePatterns without two entries claiming the same weight. Files compete with directories and ties break toward the deeper path, so the name that comes back is the most specific one that still accounts for the bytes: a vector store is reported at its own directory rather than at the repository root, and a lone oversized archive as the file rather than the directory holding it. The totals cover the whole corpus, not the prefix that had been read when the budget tripped, because they come from the walk rather than from the read. Where the walk hit its own maxFiles cap the message says so, since the totals are then partial. Refs clay-good#504.
586320a to
7865ba3
Compare
|
The red Windows job is two 30-second timeouts, not assertion failures, and neither test touches what this PR changes. From the run's own Both are long filesystem-heavy tests — a full import and a watcher parity gate — and the same run already reports This PR adds one constant to I cannot re-run the job from a fork, so I have pushed the same tree under a fresh SHA to get a clean result. If it comes back red on the same two files I will dig into them properly rather than assume flake. |
What & why
✗ Analysis failed: Project fingerprint byte budget exceeded (1073741824)names no path. The one question it raises, which files did this, is the one it does not answer, so the only way forward was to scan the filesystem by hand until the weight turned up. That is what #504 reports having to do.The information was already in memory and being thrown away. By the time the read loop trips the budget,
FileWalkerhas stat'd every admitted file, so the sizes are sitting inwalk.files. The error now spends them:Three things about that list are decisions rather than mechanics:
tmp/vecheck_datais reported there rather than attmp, and a lone oversized archive is reported as the file rather than as the directory that happens to hold it.The totals cover the whole corpus rather than the prefix that had been read when the budget tripped, because they come from the walk and not from the read. Files are read in sorted path order, so the file that trips the budget is just the one that happens to cross the line; a list built from the read so far would be an accident of the alphabet. Where the walk hit its own
maxFilescap first the message says so, since the totals are then partial.Refs #504.
Scope
The issue proposes four things. This PR does the two that are diagnostics:
SKIP_EXTENSIONSwith.lance,.parquet,.arrow(proposal 3): deliberately not here. It is not a diagnostic, it is a change to what every project indexes by default, and it fails silently in the direction that hides data rather than reports it. Someone whose repository legitimately contains.parquetwould quietly stop having it analyzed with no message saying so. That deserves its own PR and a maintainer's call, and the diagnostic above makes the need for it visible in the meantime. Happy to open it separately if you want it.openlore doctorintegration (proposal 4): also left out, for the same reason plus size.doctorinspects infrastructure and would need to run a walk to say anything here.So this is not a full
Fixes #504, which is why the commit saysRefs. Close the issue on merge if you consider the reporting half sufficient, or keep it open for proposals 3 and 4.Checklist
npm run typecheckpassesnpm run lintpassesnpm run test:runpassessrc/core/analyzer/,src/core/generator/stages/, orsrc/core/services/mcp-handlers/: rannpm run test:e2edocs// specs) if behavior or counts changedNotes on the boxes above, because two of them need qualifying:
npm run test:run. 10134 passed, 60 failed, 12 skipped. The 60 sit in 8 files (blast-radius,enforce,impact-certificate,git-hooks,pi-surface,file-walker-corpus-boundary,atomic-store,progress) and all of them fail the same way onmain: the base run in a clean worktree gives 10125 passed, 61 failed, 12 skipped in the same 8 files. Total tests differ by exactly the 8 added here (10198 → 10206). Running that set of 8 files alone gives byte-identical results on both trees (219 passed, 58 failed), so the 60-vs-61 wobble is test-ordering flakiness in the full run, not this diff. They look environmental to my machine:npm installskipped install scripts here, which several of those suites depend on (git hooks, native modules). I have not tried to fix them.npm run test:e2e. Not run. It needs a built semantic index (openlore embed --local), which I do not have set up, and the suite auto-skips rather than fails without one, so running it would have proved nothing. Flagging it rather than ticking it, since this does touchsrc/core/services/mcp-handlers/. Worth a maintainer run before merge.The last box is unticked because no behavior or counts changed for a passing analysis: the only difference is the text of an error that was already being thrown.
Notes for reviewers
Tests. The pair the change turns on is in
describe('fingerprint byte budget diagnostics'): a tight cap over a tree whose weight sits in one subdirectory must come back naming that subdirectory, and the same tree under a cap it fits must produce no diagnostic at all. The first fails onmainwithexpected 'Project fingerprint byte budget exceeded (1024)' to contain 'Largest contributors:'; the second passes onmaintoo, which is the point of having it, since it pins that the diagnostic is a failure-path thing only.describe('largestCorpusPaths')covers the selection rules directly: deeper-path-wins, file-over-directory, no overlapping entries, the root never named, and the limit respected.Where the constant lives.
FINGERPRINT_BUDGET_TOP_OFFENDERSwent intosrc/constants.tsper the convention in CONTRIBUTING. Note thatDEFAULT_FINGERPRINT_MAX_FILESandDEFAULT_FINGERPRINT_MAX_BYTESnext to it are still local toutils.ts; I left them alone rather than widen this diff, but they look like they belong inconstants.tstoo.Plain
Error, noterrors.*. CONTRIBUTING points at theerrors.*factories for user-facing errors, and I keptnew Error(...)here to match the two sibling throws in the same function (path escaped repository,source changed while fingerprinting) and to avoid changing what callers catch. AnOpenLoreErrorwith asuggestionfield would arguably render better; say the word and I will switch all three.Multi-line messages do survive. I checked the path this actually takes to a user:
analyze.tsinterpolateserror.messageintologger.error, which passes newlines through toconsole.error, andformatErrorinterpolates the message verbatim too. Nothing truncates at the first line.A follow-up I did not take. The corpus total is known before a single byte is read, so the budget could fail immediately instead of after reading up to a gigabyte. I left the check where it is because moving it changes which error wins for a repository that is both over budget and has, say, a path escaping the root, and that is a behavior change rather than a diagnostic. Easy to do separately if you want the speed.