hypgrep enters the dependency set behind an overrides pin that holds the exact LLP 0222 hyparquet floor

[philcunliffe]

Root cause

hyp query grep (LLP 0264 #dependency) both builds and reads hypgrep sidecar indexes, so hypgrep is a plain root dependency. Installed as-is it is not inert: hypgrep 0.5.1 pins hyparquet 1.27.1 and hyparquet-writer 0.16.4, and npm resolves both privately under node_modules/hypgrep:

+-- hypgrep@0.5.1
| +-- hyparquet-writer@0.16.4
| | `-- hyparquet@1.27.1 deduped
| `-- hyparquet@1.27.1

That second, older hyparquet is the exact failure LLP 0222 #hyparquet-floor exists to prevent. icebird's converter pushes bare relational bounds, correct only because hyparquet >= 1.28.2's matchFilter rejects null cells in $lt/$lte/$gt/$gte. On 1.27.1 those coerce a null cell to 0 and the bound leaks NULL rows. The floor is exact, not a minimum, and the failure is wrong rows rather than an error.

The fix

hypgrep 0.5.1 into dependencies, and a root overrides entry beside icebird's pinning hypgrep's hyparquet to 1.28.2 and its hyparquet-writer to 0.16.6. Both then dedupe to the copies already at the root, so the dependency joins the read path without adding a resolved copy of anything:

+-- hyparquet@1.28.2
+-- hypgrep@0.5.1 overridden
| +-- hyparquet-writer@0.16.6 deduped
| `-- hyparquet@1.28.2 deduped
`-- icebird@0.8.22 overridden
+-- hyparquet-writer@0.16.6 deduped
`-- hyparquet@1.28.2 deduped

hyparquet-writer stays an optionalDependency: the override pins a version inside hypgrep's subtree, it does not promote the root entry. Verified npm install --omit=optional still boots and reads (hyp --version, hyp query sql "select 1 as x", and import('hypgrep') all fine; the prepare step fails there on master too, because typescript 7 ships its platform binary as its own optionalDependency, unrelated to this change).

The test that proves it

test/core/hyparquet-floor-pin.test.js, a lint on a repository property in the idiom of repo-scratch-hygiene.test.js. Both halves are load-bearing and both redden if the override is dropped:

• Manifest half. Reads the floor off the root hyparquet pin rather than writing it twice, then requires every root dependencies entry that declares its own hyparquet or hyparquet-writer to either agree with the floor or be named in overrides. The next below-floor dependency reddens here instead of in a row count. Also pins that hypgrep is in dependencies and that hyparquet-writer is optional-only.
• Resolved half.npm ls in assertion form, over the installed tree, skipped when node_modules is absent. This is the part a manifest read cannot give you: an override nested under the wrong key, or naming a version no longer at the root, still parses as valid JSON and still installs a private copy.

Observed failing on master (hypgrep belongs in dependencies), and observed failing in the counterfactual (hypgrep added without the override) with both halves red:

not ok 3 - every read-path dependency that carries hyparquet is held at the floor
hypgrep declares hyparquet@1.27.1, not 1.28.2, and no overrides entry pins it
not ok 4 - the read path resolves the one root hyparquet, not a nested copy
hypgrep resolved a private hyparquet@1.27.1
hypgrep resolved a private hyparquet-writer@0.16.4

Verification

• npm test: 4487 pass, 0 fail, 1 skipped.
• npm run typecheck: clean.
• npm pack --dry-run: file set byte-identical to master, 932 files (diffed the two --json file lists).

Note on "exactly one resolved copy"

The acceptance criterion asks for one npm ls hyparquet copy tree-wide. That is already false on master, before this change, and for reasons outside the read path: hyparquet-writer@0.16.6 pins its own hyparquet@1.28.1, and the optional hypvector@0.2.2 brings hyparquet@1.26.2 plus hyparquet-writer@0.16.1 with hyparquet@1.26.1. All are exact pins, so collapsing them needs overrides on those packages too, which forces untested version combinations on the write and vector paths and is not what this issue asked for. What this change does hold, and what the test asserts, is the read path LLP 0222's floor governs: hypgrep adds no resolved copy of anything, and root dependencies (hyparquet, icebird, hypgrep) all resolve the single 1.28.2. The pre-existing write-side and vector-side copies are worth their own issue.

Fixes#870

[philcunliffe]

Neutral review round: PR #877 @ a77048a6

Verdict: the dependency change is correct as written; the test that guards it was weaker than its own header claimed. Five findings (1 medium, 4 low), four fixed and pushed as 7588c371. No change to package.json was needed.

The manifest change checks out

Independently verified against the reviewed head:

• hypgrep@0.5.1 declares hyparquet@1.27.1, hyparquet-compressors@1.1.1, hyparquet-writer@0.16.4. The overrides entry forces 1.28.2 / 0.16.6 and npm dedupes both to the root copies. npm ls hyparquet on a fresh install of the post-PR manifest shows hypgrep@0.5.1 overridden with hyparquet@1.28.2 deduped and no private copy.
• API compatibility of the forced bump: the only file differing between hyparquet 1.27.1 and 1.28.2 is src/filter.js (the $lt/$lte/$gt/$gte null guard, plus a widening of $in). hypgrep uses $in only on a scalar STRING column, so the bump is behaviour-neutral for it. Its deep import of hyparquet/src/convert.js#DEFAULT_PARSERS is still a declared subpath in 1.28.2's exports. hyparquet-writer 0.16.4 -> 0.16.6 is additive for the symbols hypgrep imports.
• Functional probe with the overridden versions: built a 2000-row parquet, ran createIndex, then parquetFind; results matched a brute-force scan exactly.
• The "stays optional" invariant is intact: --omit=optional still pulls hyparquet-writer in, but it did so before this PR too (icebird hard-depends on it), so nothing is newly broken.

Findings

1. medium - test/core/hyparquet-floor-pin.test.js:72 (reviewed head): the "manifest half" was not a manifest read, so the whole gate no-opped on a clean checkout.
The header split the file into a manifest half and a resolved half, but test 3 reached into node_modules/<name>/package.json via installedDependencies(), and returned undefined (skipped, silently) when the package was not installed. Test 4 returned green early for the same reason. Reproduced: ran the suite against a tree with hypgrep absent - all four tests green, nothing about hypgrep or the override ever examined. The comment at :100-101 ("the manifest half above is the part that runs everywhere") was therefore false, and nothing anywhere asserted that overrides.hypgrep exists at all: deleting the entry reddened only where deps happened to be installed.

Fixed. The override is now asserted straight off the root package.json in test 2 (overrides.hypgrep?.hyparquet === FLOOR, overrides.hypgrep?.['hyparquet-writer'] === WRITER_PIN), so a dropped or misspelled entry reddens on any checkout. Tests 3 and 4 now t.skip() visibly instead of returning green, and the header says which half is which and why the resolved half cannot be a manifest read (this repo checks in no lockfile, so a dependency's own declaration only exists under node_modules).
Counterfactual with overrides.hypgrep deleted, no reinstall: not ok 2, not ok 3. With node_modules/hyparquet hidden: tests 3 and 4 report # SKIP, not pass.

2. low - :106 (reviewed head): the nested-copy scan was depth-1, so the leak it advertises was invisible one level deeper.
installedVersion(dep, name) looked only at node_modules/<dep>/node_modules/<pinned>. A copy at node_modules/icebird/node_modules/hyparquet-writer/node_modules/hyparquet would never have been seen, which is a plausible shape for an override that half-took.

Fixed.nestedCopies() now walks each root dependency's whole node_modules subtree (with packagesIn() expanding @scope/name correctly). Verified by planting hypgrep/node_modules/hyparquet-writer/node_modules/hyparquet@1.27.1: the test fails with hypgrep/node_modules/hyparquet-writer/node_modules/hyparquet is hyparquet@1.27.1, not 1.28.2. Scope is still root dependencies only, i.e. the read path, which the header documents and which matches the PR's stated boundary; the pre-existing hyparquet-writer and hypvector copies stay out of scope as the PR body says.

3. low - :83 (reviewed head): only the child's dependencies block was inspected.
A future read-path dependency declaring a below-floor hyparquet under optionalDependencies or peerDependencies produced zero offenders, even though npm resolves copies for both.

Fixed.installedDeclarations() merges peerDependencies, optionalDependencies and dependencies (plain deps last, so they win on conflict). Suite stays green with the merge in place.

4. low - :109 (reviewed head): any nested copy was flagged, including a benign one at the pinned version.
if (version) nested.push(...) would fail CI with a message asserting a floor violation for a node_modules/<dep>/node_modules/hyparquet@1.28.2 that npm can leave behind after a targeted npm install <pkg> with no dedupe pass.

Fixed. Nested copies are compared against PINNED[dep] and skipped when they match; the failure message now names both versions (... is hyparquet@1.27.1, not 1.28.2). Confirmed the planted on-pin hyparquet-writer@0.16.6 in the depth test above is correctly ignored while the off-pin hyparquet beneath it still fails.

5. low - :84 (reviewed head): declared[dep] === pin compares a declared spec to an exact version by string equality.Considered, not changed. A future dependency declaring "hyparquet": "^1.28.2" is reported as an offender and pushed toward an overrides entry it does not strictly need. This is a real false positive, but it fails closed (the wrong direction would be silently trusting a range), the message tells a human exactly what to look at, and the alternative is either vendoring semver range logic into a lint or trusting a spec string without checking what resolved. Left as is deliberately; flagging it here so the next round does not re-derive it.

Checks

Against the pushed head 7588c371, with a full npm install:

• npm test: 4487 pass, 0 fail, 1 skipped.
• npm run typecheck: clean.
• No package.json change in this round (git diff a77048a6..HEAD -- package.json is empty); the only file touched is test/core/hyparquet-floor-pin.test.js, +114/-31.
• Style: no semicolons, no em dashes, JSDoc types only.

[philcunliffe]

Neutral review round: 7588c37 - findings (5 fixed, 1 open, 1 informational)

Reviewed package.json (the hypgrep dependency + overrides entry) and the new test/core/hyparquet-floor-pin.test.js. The override itself is right: I built a probe manifest from this PR's dependencies/optionalDependencies/overrides, ran a real npm install, and hypgrep's hyparquet and hyparquet-writer both dedupe to the root 1.28.2 / 0.16.6 with no private copy, exactly as the PR body's tree shows. Deleting the override reddens tests 2, 3 and 4 with accurate messages. npm test (4487 pass, 0 fail) and npm run typecheck are green on a faithful install.

The findings are all in the gate, not in the pin.

Fixed and pushed (415587c)

1. test/core/hyparquet-floor-pin.test.js:63 (MEDIUM) - the floor assertion pinned the root instead of reading it.
assert.equal(FLOOR, '1.28.2') sat three lines under a comment saying the floor is read from the root pin "rather than written twice". Bumping hyparquet to any later release would redden npm test with 1.28.2 is a floor and an exact pin, not a range, while LLP 0222 #hyparquet-floor states the requirement as hyparquet >= 1.28.2. Split into HYPARQUET_FLOOR (the LLP constant, a minimum) and ROOT_PINS (read off the manifest); the root pin is now asserted exact-shaped and at or above the floor.

2. :115, :142 (MEDIUM) - exact string equality misdiagnosed versions above the floor.
declared[dep] === pin reported any non-identical version under an unpinned older copy resolves beside the floor and answers relational bounds wrong. Bump the root pin to 1.28.3 and icebird's declared 1.28.2 is flagged with that message, though 1.28.2 is the floor and answers correctly. Comparison is now numeric against the floor (atOrAboveFloor, 17-case comparator probe including 1.9.0 vs 1.28.2 so it is not lexicographic), and the message names the floor the declaration is below.

3. :116 (MEDIUM) - the writer half of the lint could switch itself off.
overrides[name]?.[dep] === pin with WRITER_PIN undefined evaluates undefined === undefined and continues for every package with no overrides entry. Remove hyparquet-writer from optionalDependencies and that half of the check silently stops running. A missing root pin is now reported as an offender (once, not per dependency).

4. :203 (MEDIUM) - the nested-copy walk skipped symlinked packages entirely.
entry.isDirectory() is false for a symlink under withFileTypes, so under pnpm or npm link hypgrep the walk found nothing. Demonstrated: with node_modules/hypgrep/node_modules/hyparquet a symlink to a 1.27.1 store copy, the old test 4 passes green and the new one fails with hypgrep/node_modules/hyparquet is hyparquet@1.27.1, below the floor 1.28.2. Fixed by accepting symlinked entries, with a realpath seen set so a self-referential link cannot loop (probed).

5. :111 (MEDIUM) - the resolved half proved nothing about a dependency that is not installed.
if (!declared) continue means an override target absent from node_modules is skipped silently. That is not hypothetical: any node_modules installed before this PR has no hypgrep, so both resolved-half tests pass vacuously on exactly the entry they exist to gate (reproduced against this repo's current tree). A root dependency with an overrides entry that is missing from an otherwise-installed tree is now reported as a stale tree, with run \npm i`in the message. Note for reviewers: a checkout with a pre-PRnode_moduleswill redden here untilnpm iis rerun; CI runsnpm ibeforenpm test, and a checkout with no node_modules` still skips.

Open - needs a human decision, not fixed here

6. package.json:87 (HIGH) - the overrides entry does not survive publication.
npm honours overrides only for the root project. npm i -g hypaware and npx hypaware, both documented in README.md:45,49 and both in the release checklist, install hypaware as a dependency, where the field is ignored and hypgrep gets its own hyparquet@1.27.1 and hyparquet-writer@0.16.4 under node_modules/hypgrep/node_modules - the exact private below-floor copy this PR exists to remove, plus the resolved copies the PR body says it adds none of. So the invariant holds in this checkout and in CI, and not in any user install.

Mitigating: hypgrep's only pushdown is filter: { ngram: { $in: [...] } } over a STRING column, $in does not go through the null-coercing relational path, and hypaware's own icebird read path still resolves the root 1.28.2 - I found no wrong-rows route today. And nothing imports hypgrep yet (see below), so nothing exercises it at all.

Not fixed because both remedies are design calls above a review round. LLP 0264 #dependency already names the durable one ("widening the range upstream in hypgrep"); the other is shipping an npm-shrinkwrap.json, the only lockfile npm honours for consumers of a published package, which contradicts this repo's deliberate no-lockfile property (stated in this very test file). What I did do is stop the file claiming more than it proves: the header now records that the override governs root installs only and is not a substitute for the upstream widening.

Related: a check on this would have to pack and install the package, which does not belong in the traditional suite. If you want it gated, it is an acceptance-tier candidate (published_install_floor_check), not a test/** lint.

Informational

7. package.json:74 - hypgrep is a hard runtime dependency no code imports yet.grep -rn hypgrep src/ hypaware-core/ bin/ hits nothing; only llp/0264 and llp/0265 mention it. Until LLP 0265's implementation lands, every install pulls hypgrep (and, per finding 6, its duplicated hyparquet family) for a code path that does not exist. Presumably deliberate sequencing - worth a line in the PR description.

Checked and cleared

• overrides.hypgrep coexists with hypgrep as a direct dependency; no npm conflict-rule violation, install succeeds and dedupes as claimed.
• No API breakage forcing hypgrep from hyparquet 1.27.1 to 1.28.2: every module under hypgrep/src link-checks against 1.28.2, including the deep hyparquet/src/convert.js import.
• The nested walk costs about 1ms on a real node_modules; no test-runtime concern.
• @ref anchors LLP 0222#hyparquet-floor and LLP 0264#dependency both exist and still say what the annotations claim; the 0222 gloss was reworded from "exact floor" to "floor" to match the doc.
• The PR's "hyparquet-writer stays optional" claim is true of the manifest only: --omit=optional still installs it, because icebird@0.8.22 declares it as a hard dependency. Not a regression, it was already so on master.

[philcunliffe]

Neutral triage: PR #877 @ 415587c3

Review rounds are exhausted with two residual findings. Both judged non-blocking from the code at head; deferred to #910.

• Overrides pin does not survive publication (round 2, finding 6, labeled HIGH): real gap in published installs, but no production defect path today. Nothing imports hypgrep; icebird declares hyparquet@1.28.2 exactly, so the relational-pushdown read path holds the LLP 0222 floor in every install shape; hypgrep's only pushdown is $in over a STRING column, which never reaches the null-coercing relational path. The durable fix (upstream range widening per LLP 0264, or a shrinkwrap decision) is tracked in Follow-up: deferred review findings from PR #877 #910 and should land before LLP 0265's implementation makes hypgrep a live import.
• hypgrep is a hard dependency no code imports yet (round 2, finding 7, informational): install weight and sequencing only; resolves when LLP 0265's implementation lands.

The PR can merge safely as-is; findings 1-5 of both rounds were fixed and pushed (7588c371, 415587c3).