Summarize a dictionary or a MAT-file from a scan the caller already has - #30
Merged
Merged
Conversation
`summarizeSlddScan` and `summarizeMatScan`, the exact analogue of `summarizeParsedModel` for the other two file kinds: a `FileSummaries` holding the one file, built from a `scanSldd`/`scanMat` result instead of from bytes. A caller that has already scanned a data file could not get its summary without scanning it again. data-explorer-vscode's cheap tier is that caller — it wants both the raw `refs` a scan carries and the summary out of one read — and the second scan measured 46% of that tier's per-dictionary CPU: 14.6 ms of 31.5 ms on a 20,000-entry dictionary, spent deriving a `names` list it was already holding. `summarizeFiles`'s sldd and mat branches now route through the new functions, as its model branch already routes through `summarizeParsedModel`. It still scans the bytes itself — it takes bytes, not scans — and only stops open-coding the summary, so the two ways to reach one summary share an implementation rather than agreeing. Which weakens the obvious test, and the new sweep says so: an equality between the two paths over every committed `.sldd` and `.mat` cannot fail on the shared code, so every swept file is ALSO compared against a summary derived from the full readers `readSlddContent` and `parseMat`. Verified by mutation — without that second assertion, dropping `filter(Boolean)` from the summary left the whole sweep green. The constructed cases cover what no fixture has: references in both spellings and mixed case, unnamed entries around a named one, the basename key rule, a dictionary whose fast path refuses and whose full read recovers, a truncated MAT, and the two files no reader can read at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
summarizeSlddScanandsummarizeMatScan— the analogue ofsummarizeParsedModelfor the other two file kinds. Each takes a
scanSldd/scanMatresult instead ofbytes and returns the
FileSummariesfor that one file.Why
A caller that has already scanned a data file could not get its summary without
scanning it again.
data-explorer-vscode's cheap tier is that caller: it wants theraw
refsa scan carries and the summary, out of one read. The second scan measured46% of that tier's per-dictionary CPU — 14.6 ms of 31.5 ms on a 20,000-entry
dictionary — spent deriving a
nameslist it was already holding.What changes here
summarizeFiles's sldd and mat branches now route through the new functions, as itsmodel branch already routes through
summarizeParsedModel. It still scans the bytesitself — it takes bytes, not scans — and only stops open-coding the summary. So the
two ways to reach one summary now share an implementation rather than agreeing.
That weakens the obvious test, and the new sweep says so: an equality between the two
paths over every committed
.slddand.matcannot fail on shared code. So everyswept file is also compared against a summary derived from the full readers
(
readSlddContent,parseMat). Verified by mutation — without that second assertion,dropping
filter(Boolean)from the summary left the whole sweep green.The constructed cases cover what no fixture has: references in both spellings and
mixed case, unnamed entries around a named one, the basename key rule, a dictionary
whose fast path refuses and whose full read recovers, a truncated MAT, and two files
no reader can read at all.
Verification
npm run typecheckcleannpm test— 159 files / 4741 tests passing (was 158 / 4676 onmain)npm run buildleavesdist/byte-identical to what is committed here, anddist/index.d.tsdeclares both new exports (the prebuiltdist/ships becausegit-dependency installs skip
prepare)