Shared-scan graph projection: one scan per contract, declarative rule predicates (LLP 0095/0096)

[philcunliffe]

What

projectGraph executed every contract rule as an independent full SQL scan of the source dataset, and the LLP 0026 aux filter prepended attributes (19% of table bytes) to every rule's SQL and JSON-parsed it once per rule per row. With the ai-gateway contract's 25 rules, one projection = 25 full table sweeps.

Observed on a production hypaware-server deployment: a single org-scoped graph project over a ~700k-row / ~1.3GB source read ~35GB from disk and had not finished after 45 minutes (single core pegged) before being aborted. The same corpus projects in seconds locally, which is why this was never noticed: local installs are just earlier on the same rules x table-size curve. Full evidence in LLP 0095.

How

Settled in LLP 0096:

• One shared scan per contract. Rules declare { columns, where } instead of raw SQL; the engine scans the source once with the union of declared columns and evaluates each rule's predicate in JS per row before toRow. where is the AND of the three shapes the rules actually use (eq, in, likePrefix), with SQL null semantics.
• Raw-SQL rules stay supported, run standalone, grouped by identical SQL text. Two reasons to keep the hatch: contracts elsewhere migrate on their own schedule (hypaware-server's github contract, context-graph-enrich), and the two content_text LIKE 'prefix%' Skill surfaces deliberately stay pushed down so the table's largest column never enters the shared scan's materialization.
• The aux filter moves to the contract as rowFilter, evaluated once per row on both paths (previously: per rule per row). The registry enforces that raw-SQL rules under a rowFilter select its columns themselves.

Net effect at server scale: 25 full scans become 1 slim scan + 2 pushed-down prefix scans, and 25x fewer attributes parses.

Tests

• New equivalence test projects the migrated contract and a mechanically-built raw-SQL twin over a fixture hitting every predicate shape plus the aux filter, and asserts row-identical node/edge output - this pins the JS evaluator to the SQL engine's semantics.
• Registry validation coverage for the new shapes (exactly-one-of, predicate shapes, rowFilter, raw-SQL-selects-filter-columns).
• Existing e2e / idempotence / provenance / skills-surfaces tests pass unchanged; context_graph_projects_rows smoke passes; typecheck clean.

Notes

• Complementary to LLP 0086-0092: automatic projection of derived datasets (design) #279 (automatic projection, LLP 0086-0092, design-only): a 15-minute auto-projection tick needs cheap runs; this PR is effectively a prerequisite for that design at any accumulated history. Incremental (watermarked) projection stays out of scope here, as LLP 0096 records.
• hypaware-server's github contract keeps working via the raw-SQL path, no lockstep needed.

🤖 Generated with Claude Code

[philcunliffe]

Dual-agent review — request_changes

• Verdict: request_changes
• Risk class: low
• Auto-merge advisory: 👎 thumbs down — verdict is request_changes; needs human-gated follow-up

Advisory only: no merge was attempted. Head reviewed: 209e250.

Risk capstone

Cross-reference: reviewer findings vs high-risk surfaces

Source	Finding (severity, evidence)	Intersects
Claude	inline import() types violate CLAUDE.md (minor, project.js:55,110,216,247)	Targets (project.js), not a Risks-section surface

Codex review

Fix Validations

Per-rule graph projection scans

• Status: correct
• Evidence: hypaware-core/plugins-workspace/context-graph/src/project.js:84, hypaware-core/plugins-workspace/context-graph/src/project.js:91, hypaware-core/plugins-workspace/context-graph/src/project.js:118, test/plugins/context-graph-project-e2e.test.js:457
• Assessment: Declarative rules now share one union-column scan per contract, while raw SQL is grouped by identical SQL. The new e2e SQL-twin test validates row-identical output for the migrated ai-gateway contract.

LLP 0026 aux exclusion preservation

• Status: correct
• Evidence: hypaware-core/plugins-workspace/ai-gateway-graph/src/graph_contract.js:470, hypaware-core/plugins-workspace/context-graph/src/project.js:100, hypaware-core/plugins-workspace/context-graph/src/project.js:128
• Assessment: The aux filter is now applied before rule fanout on both shared-scan and raw-SQL paths. Current ai-gateway raw rules explicitly project attributes.

Findings

2) Contract & Interface Fidelity

• Severity: major
• Confidence: high
• Evidence: hypaware-core/plugins-workspace/context-graph/src/contract-registry.js:94, hypaware-core/plugins-workspace/context-graph/src/contract-registry.js:96, test/plugins/context-graph-contract.test.js:158, hypaware-core/plugins-workspace/context-graph/src/project.js:128
• Why it matters: The registry promises raw SQL under a rowFilter actually selects filter columns, but rule.sql.includes(col) accepts false positives like WHERE attributes IS NOT NULL or SELECT other_attributes, so keep(row) can run with a missing column and silently let filtered rows through.
• Suggested fix: Validate the projected SELECT list, not the whole SQL string, and add negative tests for WHERE attributes... and other_attributes; also consider checking result.columns before applying keep.

9) Test Evidence Quality

• Severity: minor
• Confidence: high
• Evidence: hypaware-core/plugins-workspace/context-graph/src/project.js:233, test/plugins/context-graph-project-e2e.test.js:453, hypaware-core/plugins-workspace/ai-gateway-graph/src/graph_contract.js:219, hypaware-core/plugins-workspace/ai-gateway-graph/src/graph_contract.js:232
• Why it matters: The test comment claims the equivalence fixture exercises likePrefix, but the ai-gateway prefix surfaces stayed raw SQL, leaving the new JS likePrefix evaluator without direct coverage.
• Suggested fix: Add a small synthetic declarative-rule projection test with where: { likePrefix: ... }, including null/non-string/non-matching rows.

No Finding

1. Behavioral Correctness
2. Change Impact / Blast Radius
3. Concurrency, Ordering & State Safety
4. Error Handling & Resilience
5. Security Surface
6. Resource Lifecycle & Cleanup
7. Release Safety
8. Architectural Consistency
9. Debuggability & Operability

Evidence Bundle

• Changed hot paths: projectGraph shared scan and raw grouping at hypaware-core/plugins-workspace/context-graph/src/project.js:57, :84, :118; predicate evaluation at hypaware-core/plugins-workspace/context-graph/src/project.js:221; contract validation at hypaware-core/plugins-workspace/context-graph/src/contract-registry.js:56.
• Impacted callers: CLI projection via hypaware-core/plugins-workspace/context-graph/src/command.js:42; contract registration capability via hypaware-core/plugins-workspace/context-graph/src/index.js:62; raw legacy contract shape still present at hypaware-core/plugins-workspace/context-graph-enrich/src/contract.js:54.
• Impacted tests: test/plugins/ai-gateway-graph-contract.test.js:182; test/plugins/context-graph-contract.test.js:137; test/plugins/context-graph-contract.test.js:149; test/plugins/context-graph-project-e2e.test.js:457.
• Unresolved uncertainty: I did not run the suite; review used the supplied diff plus targeted rg/line checks. External hypaware-server contracts mentioned in the PR are outside this worktree.

Claude review

Claude review

Reviewed head 209e250. Focus: equivalence of the shared single scan vs. the prior per-rule scans (nodes/edges must be identical), totality of the declarative predicates, aux-filter semantics, and the withholding/export-seam invariant.

Equivalence risks checked and cleared:

• Aux-filter semantics identical. Old if (auxKindOf(r.attributes)) return null vs new keep(r){ return auxKindOf(r.attributes) === null }. auxKindOf returns str(claude.aux_kind), and str maps ''/non-string to null and never returns undefined — so it yields exactly non-empty-string | null. === null is therefore identical to the old truthiness test at every value (including empty-string aux_kind and stringified attributes).
• Predicate<->SQL mapping is exact. Every migrated WHERE maps 1:1 to eq/in (part_type/tool_name/role); matchesPredicate uses strict !== for eq and typeof v !== 'string' guards for in/likePrefix, so a null/absent column never matches — mirroring SQL. Heavy content_text LIKE 'prefix%' guards (Skill marker/slash surfaces) correctly stay raw SQL for server-side pushdown, and those raw rules now select attributes themselves (registry-enforced), so the aux filter still applies to them.
• Row-order independence holds. The shared scan changes Map insertion order (row-major instead of rule-major), but mergeRow/propsValueWins resolve conflicts by earliest first_seen then stable-JSON tie-break, so the merged node/edge set is order-independent. The new shared-scan is row-identical to per-rule SQL e2e test passes and exercises eq/in/likePrefix + aux + null columns.
• Column-union completeness. columns union + predicateColumns(where) + rowFilter.columns covers every field each toRow reads (spot-checked File/touched, Program/invoked, Skill surfaces). toRows read named fields only, so extra union columns in the row are inert.
• Withholding/export-seam (LLP 0070/0071) not implicated. Graph projection is a purely local read of ai_gateway_messages to build local node/edge tables; it is not the sink/export path, and there are no withholding references in the projection code. Dropping per-rule WHERE does not widen any export.

Inline import('./types.js') type annotations violate the repo type-import rule

• Severity: minor
• Confidence: 85
• Evidence: hypaware-core/plugins-workspace/context-graph/src/project.js:55, 110, 216, 247
• Why it matters: CLAUDE.md states "Never use inline import('...') types. Declare type imports at the top of the file with @import JSDoc comments, then reference the bare names." The PR adds four inline import('./types.js').ContractRule / .RulePredicate annotations, and the file already has an @import { Contract, GraphRow } from './types.js' block at the top that these belong in.
• Suggested fix: Extend the existing top-of-file @import to { Contract, GraphRow, ContractRule, RulePredicate } from './types.js' and use the bare names at the four sites.

---

Reports: /Users/phil/workspace/hypaware/.git/worktrees/wt/dual-review/pr-291

[philcunliffe]

neutral review — round 1 · verdict: request-changes · adopted PR (LLP 0025, full-heal)

Reviewed head 209e250 (Codex + Claude, independent). The central concern — scan equivalence — is cleared: the shared single-scan projection is behavior-preserving. The bit-identical equivalence e2e passes, aux-filter semantics are provably identical to the old truthiness test, every migrated WHERE maps 1:1 to eq/in with strict null-safe guards, row-major processing is order-independent via first_seen + stable-JSON tie-break, and there is no withholding/export-seam impact (LLP 0070/0071) — projection is a purely local read of ai_gateway_messages, not the sink/export path.

Findings — all non-blocking. None is a true blocker (no production defect in the shipped code); handed back for your call.

1. major (robustness, not a live defect) — hypaware-core/plugins-workspace/context-graph/src/contract-registry.js:94-96. The guard "raw SQL under a rowFilter must select the filter's columns" uses a loose textual rule.sql.includes(col). It accepts false positives like WHERE attributes IS NOT NULL or SELECT other_attributes. Scenario: a future raw rule that mentions but does not actually SELECT attributes passes registration, then keep(row) reads undefined → auxKindOf(undefined) treats it as non-aux → aux (security-monitor) rows silently leak into the graph as Session/App/Tool nodes. Every current raw rule does SELECT attributes, ..., so no live leak — but the guard should validate the projected SELECT list, not the whole SQL string.

2. minor (coverage) — test/plugins/context-graph-project-e2e.test.js:453 + project.js:221. The equivalence test comments claim likePrefix coverage, but the ai-gateway prefix surfaces stayed raw SQL, so the JS matchesPredicate likePrefix branch has no direct evaluation coverage. Add a synthetic declarative rule with where:{likePrefix} incl. null / non-string / non-matching rows.

3. minor (style, CLAUDE.md) — hypaware-core/plugins-workspace/context-graph/src/project.js:55,110,216,247. Four inline import('./types.js').ContractRule / .RulePredicate annotations violate "never use inline import() types; declare at top via @import." The file already has an @import ... from './types.js' block — extend it and use bare names.

LLP consistency: good. LLP 0095 (Issue, Active) and 0096 (Decision, Accepted) are well-formed and accurately describe the implementation; in-code @ref LLP 0096#decision annotations resolve, and the LLP 0026 aux-filter ref was correctly relocated to the rowFilter. The two identical SELECT_COMMITTED cross-repo rules now collapse to one scan — a free improvement.

Design note (not a finding): columns is now decoupled from toRow, and the equivalence e2e reuses the same columns list for its SQL twin, so it would not catch a future author omitting a toRow-read column from columns. The per-rule toRow unit tests are the backstop.

---

Why neutral did not push fixes: you are actively iterating on perf/shared-scan-projection (head committed ~15 min before this review, working tree dirty). Full-heal mode could push these fixes to the branch head, but that would collide with your in-flight local work, so neutral handed the findings back instead. Fold them in (or consciously accept and merge over) at your discretion — none block merge. A new push re-opens review at the new head next tick.

[philcunliffe]

neutral review — round 2 · verdict: clean → approved · adopted PR (LLP 0025, full-heal)

Round 1 (at 209e250) found 1 major + 2 minor, all non-blocking. neutral healed them in 6332b31 (fast-forward from the round-1 head) and independently re-reviewed the fix delta. All three findings resolved; no new issues.

Fix confirmation (verified against the committed tree + local tests):

• major — raw-rule column guard RESOLVED. The loose sql.includes(col) was replaced with rawSqlProjectsColumn, which parses only the top-level SELECT projection (paren-depth-0 SELECT..FROM, top-level comma split, per-item output-name reduction). Verified both directions: false-accepts closed (WHERE attributes IS NOT NULL, SELECT other_attributes, subquery-only attributes, col AS y alias, substring identifiers all reject); legitimate forms accept (bare col, x.attributes, foo AS attributes, *, table.*). All 4 real attributes rowFilter rules still register (no false-reject). Reject path preserves the original throw.
• minor — likePrefix coverage RESOLVED. matchesPredicate exported and directly tested for matching-prefix / non-matching / null / non-string.
• minor — inline import() types RESOLVED. All four inline annotations removed; @import block extended to { Contract, ContractRule, GraphRow, RulePredicate }.

Checks at this head: typecheck + test green, MERGEABLE. Tests locally: 28/28 (context-graph contract/project/e2e) + 46/46 (ai-gateway-graph, exercising the real attributes rowFilter incl. "aux-tagged skill rows are excluded"). build:types clean.

Two latent, non-blocking robustness notes on the new guard (both fail closed — reject at registration, never a leak; neither reachable by any current rule, so no action needed):

1. projectionOutputName: SELECT DISTINCT attributes ... reduces to the token "DISTINCT attributes" and would false-reject a future DISTINCT rule.
2. selectProjection: string literals aren't skipped, so a literal containing a whole-word from/select before the real FROM could truncate the parsed projection early. If you ever add a rule with either shape, tighten these; the guard's JSDoc already documents the conservative stance.

Mergeable ∧ green ∧ reviewed-clean → approved. Held for you to merge — neutral does not merge or ready adopted PRs. (Reminder: origin is now ahead of your local branch by the fix commit; git pull/rebase before pushing there again.)