[dsorensen]

Summary

Metadata-aware retrieval plus persistent ranking configuration.

• frontmatter_filter on search / query / list_pages — a list of {key, op, value?} predicates (eq, exists, missing, lt, lte, gt, gte), ANDed together, validated at the operation boundary so a filtered query never silently degrades to unfiltered. Equality is index-accelerated; date comparisons work on ISO-8601 strings. New module src/core/search/frontmatter-filter.ts. Available over MCP and CLI (--frontmatter-filter '[{"key":"status","op":"eq","value":"active"}]').
• types on query — a list of page-type names validated against the active schema pack (unknown names error with the valid set), scoping a hybrid query to a set of types.
• search.source_boosts persistent config key (slug-prefix → ranking factor). Resolution ladder: built-in defaults ← config ← GBRAIN_SOURCE_BOOST env (env last, the emergency override). One shared resolver feeds both CLI and MCP.
• Query-cache correctness — the resolved boost map now participates in the cache key (knobs_hash v12 → v13, canonical serialization), and queries carrying frontmatter_filter / types / since / until skip the cache entirely, so a filtered or re-weighted lookup can never be served a stale or differently-shaped cached result. (since/until had the same pre-existing cache hole; closed here too.) One-time cache-miss spike on upgrade.

Test plan

• bun run typecheck — clean.
• bun test (frontmatter-filter, source-boost-config, search-mode, knobs-hash-reranker, cross-modal) — 185 pass / 0 fail.
• Postgres e2e parity (DATABASE_URL set, real pg16) — 11 pass / 0 fail. Exercises the JSONB bind paths PGLite hides: searchKeyword $N::text::jsonb, searchVector filter inside the HNSW candidate CTE, listPages sql.json tagged-template path.

Docs

• New guide docs/guides/typed-personal-streams.md.
• docs/architecture/RETRIEVAL.md and KEY_FILES.md updated for the resolution ladder and filter module.
• CLAUDE.md / llms-full.txt note the knobs_hash v13 bump.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MnrzTBLJ5n18BTUHmvtcVy

[dsorensen]

Closing — opened against the wrong base. This work lives on dsorensen/gbrain.

[dsorensen]

Antigravity (Gemini) review

The PR introduces robust metadata filtering (frontmatter_filter) and persistent ranking configurations. The implementation correctly isolates SQL injection risks by using strict parameter binding and an operator allowlist, ensures that filters cannot silently degrade to unfiltered results by throwing errors on validation failure, and correctly invalidates semantic query caches when filters or types are present by bumping KNOBS_HASH_VERSION.

Full analysis

1. Correctness & Architecture

• Shared SQL Compiler: Centralizing the predicate-to-SQL logic in compileFrontmatterFilter ensures that both PGLiteEngine and PostgresEngine (via searchKeyword and searchVector) remain perfectly synchronized.
• Tagged-Template Parity: The listPages operation in postgres-engine.ts correctly mirrors the logic using sql.json for containment and sql.unsafe with the FRONTMATTER_CMP_SQL allowlist.
• Fail-Closed Validation: The parseFrontmatterFilterParam function throws an explicit error if input is malformed, ensuring that users do not receive unfiltered results when they expected constrained data.

2. Security

• SQL Injection Prevention: Values and keys are safely added to the params array for positional binding ($N), and operators are strictly allowlisted.
• Schema Pack Type Validation: validateTypesAgainstActivePack throws an error if unknown types are provided, protecting the query layer from unauthorized or mistyped types. It gracefully skips validation if the active pack cannot be loaded, which aligns with existing system behavior.

3. Performance & Caching

• Query Cache Invalidation: Bumping KNOBS_HASH_VERSION from 12 to 13 and including the serialized boost map (sb=...) in the cache key properly ensures that ranking changes invalidate the cache.
• Cache Skipping: Adding hasCacheSkippingFilters correctly forces semantic cache misses for frontmatter_filter and types. While this trades off caching for correctness now, folding these into the knobsHash in the future (as noted in the comments) will be a great optimization.

4. Maintainability

• The code relies heavily on established patterns, such as the // v0... comment style for architectural changes.
• The use of resolveSourceBoostsForEngine() standardizes configuration resolution (Defaults ← Config ← Env), ensuring the CLI and MCP Server always read the ranking weights identically.

5. Testing

• The new test files test/e2e/frontmatter-filter-pg.test.ts and test/frontmatter-filter.test.ts provide excellent coverage of the new SQL logic, covering the double-encode jsonb bug class and verifying type-sensitive equality.

Verdict: The PR is highly robust and safely extends the retrieval surface. Ready to merge.

[dsorensen]

Codex review

I found one blocking correctness issue: the new frontmatter_filter / types result filters are only enforced on the SQL keyword/vector arms, but later recall stages can still inject pages that do not satisfy those filters.

Full analysis

Finding

P1: Filtered query results can be bypassed by alias-hop and relational recall

The PR correctly threads types and frontmatterFilter into the searchOpts object used by the engine keyword/vector searches (src/core/search/hybrid.ts:898-913). Those SQL arms are filtered. But hybridSearch also has non-SQL result-producing stages:

• The relational recall arm is built at src/core/search/hybrid.ts:1047 with only sourceId, sourceIds, depth, limit, and onMeta. It is later appended into RRF at src/core/search/hybrid.ts:1377.
• applyAliasHop is called after filtered retrieval/rerank at src/core/search/hybrid.ts:1486 (and also on the keyword fallback paths), but it only receives source scope.
• Inside applyAliasHop, an absent canonical alias match is fetched via engine.getPage(ref.slug, { sourceId: ref.source_id }) and pushed into the result set at src/core/search/hybrid.ts:676-682; there is no type or frontmatter predicate check.
• Relational recall has the same shape: hydrate() selects pages by slug/deleted status only (src/core/search/relational-recall.ts:112-116) before producing SearchResults.

I confirmed the alias case with an in-memory PGLite repro:

1. Create projects/mingtang with frontmatter.status = "paused".
2. Register alias hall of light.
3. Run hybridSearch(engine, "Hall of Light", { frontmatterFilter: [{ key: "status", op: "eq", value: "active" }], relationalRetrieval: false }).

The result still included:

[
  { "slug": "projects/mingtang", "alias_hit": true }
]

That page explicitly fails the requested status=active filter. The same problem applies to types: a query(types: ["decision"], ...) can still inject a non-decision alias hit, and a relational query can inject graph candidates outside the requested type/frontmatter scope.

This undermines the new typed-stream/audit use case documented in the PR, where types is described as hard scoping and frontmatter_filter is used to find contract violations. It is especially easy to miss because the PR also disables the semantic cache for filtered calls, so these wrong results are fresh, not stale cache contamination.

Suggested fix: make result filters apply to every candidate-producing path, not only the engine SQL arms. Either thread types / frontmatterFilter into applyAliasHop and buildRelationalArm/hydrate and enforce them in the fetch SQL, or apply a shared final matchesSearchFilters pass to all alias/relational-injected candidates before adaptive return, autocut, and slicing. Please add regression tests for at least:

• alias-hit page with mismatching frontmatter is not injected;
• alias-hit page with mismatching type is not injected;
• relational recall candidate with mismatching type/frontmatter is not injected.

What I Checked

• Reviewed local checkout at PR head against base f72de97943eb9dc1292a80f85d19db7e311855dc.
• Focused unit tests passed: bun test test/frontmatter-filter.test.ts test/source-boost-config.test.ts test/search-mode.test.ts (119 pass).
• Postgres frontmatter E2E was skipped as expected without DATABASE_URL: bun test test/e2e/frontmatter-filter-pg.test.ts (13 skip).
• Standalone typecheck passed: bun run typecheck.
• JSONB guardrails passed: scripts/check-jsonb-pattern.sh and bun scripts/check-jsonb-params.mjs.
• git diff --check f72de97943eb9dc1292a80f85d19db7e311855dc...HEAD passed.

[dsorensen]

Cursor review

Verdict: Ready with fixes. One P1: on balanced/tokenmax, the relational RRF arm ignores frontmatter_filter / types[], so filtered hybrid queries can still return out-of-filter graph neighbors — contradicting the PR's "never silently unfiltered" contract. Also: KEY_FILES overclaims a single listPages SQL compiler (postgres hand-rolls tagged-template SQL), unindexed comparison ops inside the HNSW CTE are a cost footgun, and a knobs-hash test title still says v12. Security binding and cache-skip/v13 boost hashing look solid. No local autofixes applied (cross-repo PR review).

Full analysis

Code Review Results

Scope: PR #2979 (pr-remote, cross-repo dsorensen/gbrain) -> master; head 3a858c9; 22 files; ~1491 executable lines changed
Intent: Metadata-aware retrieval (frontmatter_filter + types[]) and persistent search.source_boosts, with query-cache correctness (knobs_hash v12->v13; skip cache for filter/date queries).
Mode: interactive (depth:full); Stage 5c apply skipped (working tree is not the sole apply surface for a cross-repo PR review)

Reviewers: correctness, testing, maintainability, project-standards, agent-native, learnings (always); security, performance, api-contract, adversarial (conditional)

• security -- user-supplied frontmatter predicates become SQL/JSONB filters over MCP
• performance -- hybrid/HNSW SQL filters + cache-skip cost
• api-contract -- new search/query/list_pages params (CLI + MCP)
• adversarial -- large retrieval/ranking/cache delta

Cross-model adversarial peer pass: skipped (pr-remote scope).

Triage Groups

Group	Findings	Context	Preferred Resolution	Why
Filter completeness (decision-gate)	#1	New filter contract says results never silently include out-of-filter pages; relational arm bypasses it	Decide: thread filters into hydrate, post-filter before RRF, or disable relational when hasCacheSkippingFilters	One policy choice closes the silent-unfiltered hole on balanced/tokenmax
Dual listPages compiler (apply-queue)	#2	KEY_FILES/ONE compiler claim vs postgres tagged-template hand-roll	Fix KEY_FILES now; add drift test or shared visitor next	Docs lie today; code can drift later without CI signal

P1 -- High

#	File	Issue	Reviewer	Confidence
1	src/core/search/hybrid.ts:1047	Relational RRF arm ignores frontmatter_filter / types[]	adversarial	75

• feat: GBrain v0.1.0 — Postgres-native personal knowledge brain #1 — Keyword/vector arms receive frontmatterFilter/types in searchOpts, but buildRelationalArm is called with only source scope/depth/limit. hydrate selects by slug with no frontmatter/type predicates, then the list is fused on all hybrid paths. On balanced/tokenmax (relational on by default), a filtered relational question can return out-of-filter graph neighbors while still looking successful. This contradicts the PR's "filtered query never silently degrades to unfiltered" contract for the new filters. Fix: thread filters into hydrate (or post-filter relationalList before every RRF merge), or skip the relational arm when hasCacheSkippingFilters(opts) until filters are supported. Note: types on hybrid predated this PR; frontmatter_filter newly makes the gap user-facing. Validated against PR head.

P2 -- Moderate

#	File	Issue	Reviewer	Confidence
2	docs/architecture/KEY_FILES.md:69 (+ src/core/postgres-engine.ts:1335)	Docs claim one listPages compiler; postgres listPages hand-rolls SQL	project-standards, testing, maintainability	100
3	src/core/search/frontmatter-filter.ts:30	Unindexed lt/lte/gt/gte predicates pushed into HNSW CTE	performance	75

• feat: GBrain v0.2.0 — incremental sync, file storage, install skill #2 — compileFrontmatterFilter is documented as the single compiler for search + listPages on both engines. PGLite listPages uses it; postgres listPages mirrors ops fragment-by-fragment on the tagged-template path (sql.json / ? / sql.unsafe(cmp)). E2E covers today's ops, but a future compiler-only change can desync list vs search with no drift guard. Immediate fix: correct KEY_FILES (and the module header comment) to describe the dual path; follow-up: shared visitor or table-driven equivalence test.
• docs: fix first-time experience — remove fictional kindling, add recommended schema #3 — Comparison ops compile to (frontmatter ->> key) <op> value (unindexed) and are applied inside the HNSW candidate CTE. eq can use GIN @>; date/number cmp cannot. Document cost in param descriptions; optional expression indexes or post-filter strategy for cmp-only queries later.

P3 -- Low

#	File	Issue	Reviewer	Confidence
4	test/search/knobs-hash-reranker.test.ts:47	Test title still says version is 12 after assertion bumped to 13	correctness	100

• docs: expand brain schema with database architecture and OSS smoothing #4 — Cosmetic drift vs sibling search-mode tests; update the title string to v13 / source-boost fold.

Actionable Findings

#	File	Issue	Route	Notes
1	src/core/search/hybrid.ts:1047	Relational arm bypasses new result-set filters	manual -> human	Design choice among thread / post-filter / skip-arm; needs test with relational + filter
2	docs/architecture/KEY_FILES.md:69	Correct "ONE compiler" claim; add drift guard	gated_auto (docs) + manual (visitor/test)	Docs fix is mechanical; shared visitor is follow-up
4	test/search/knobs-hash-reranker.test.ts:47	Rename test title to v13	gated_auto -> downstream-resolver	One-line string fix

Pre-existing Issues

None elevated. Residual notes (not verdict-blocking): two-pass structural walk neighbor hydration historically skips some result-set filters; types SQL is bare ANY rather than schema-pack expandTypeFilter (alias/canonical expansion gap).

Learnings & Past Solutions

• No docs/solutions/ corpus in this repo.
• Adjacent patterns that this PR follows correctly: ENGINES.md #2339 jsonb bind ($N::text::jsonb / sql.json); CDX-4 knobs_hash contamination class (v13 boost fold + filter cache-skip); HNSW inner-CTE filter placement precedent (fix(mcp): seal source-isolation leak on read path (P0) #861).

Agent-Native Gaps

• MCP/CLI params are wired (frontmatter_filter on search/query/list_pages; types[] on query) with solid param descriptions and docs/guides/typed-personal-streams.md.
• Tool descriptions in operations-descriptions.ts were not updated, so agents that route from descriptions alone may miss metadata filtering.
• MCP JSON Schema for predicates is items: { type: 'object' } (opaque). Runtime validation is loud; structured properties would improve discoverability. (Validator demoted this from primary findings because description + boundary parse already fail closed.)

Coverage

• Scope: pr-remote; local HEAD OID equals PR head; inspection via git show refs/review/pr-2979-head:<path> + gh pr diff.
• Untracked excluded: .serena/*, local plan/comparison markdowns.
• depth:full — lite roster disabled.
• Reviewers completed: correctness, adversarial, testing, maintainability, project-standards, api-contract, performance, security, agent-native, learnings. Several mid-wave subagent dispatches hit usage limits; remaining personas were completed in a consolidated pass. Cross-model peer adversarial skipped (pr-remote).
• Suppressed / demoted: correctness advisory items at anchor 50 (types expandTypeFilter; dual-compiler as advisory-only before corroboration); adversarial getConfig silent fail-open and malformed-boost-JSON primary rows (validator: intentional fail-open / stderr-documented); api-contract opaque schema as primary (validator: description + loud parse suffice) — retained under Agent-Native Gaps.
• Validator: P1 feat: GBrain v0.1.0 — Postgres-native personal knowledge brain #1 and dual-compiler feat: GBrain v0.2.0 — incremental sync, file storage, install skill #2 confirmed; two P2 candidates rejected as non-defects.
• Stage 5c: no local autofixes applied (pr-remote).
• Residual risks: filtered queries always skip cache (correctness-first cost); boost config fail-open is observable mainly via stderr; packless brains skip types[] loud validation; postgres listPages dual path remains a future drift hazard until tested/refactored.
• Testing gaps: no relational + frontmatter_filter/types[] fusion test; no compiler↔postgres listPages drift guard; no tools/list schema richness snapshot; limited cache non-write assertion for filtered hybrid.

Verdict

Ready with fixes. Ship blockers for the stated contract: close #1 (relational arm must not reintroduce unfiltered pages when filters are set). Strongly fix #2 docs (+ prefer a drift test). #3/#4 are non-blocking. Security binding looks sound (params bound; key charset defense-in-depth; predicate cap 32). Cache/knobs_hash v13 and filter cache-skip are the right contamination posture.

Fix order: (1) relational filter policy + test, (2) KEY_FILES truth + drift guard, (3) optional schema/description polish and cmp-cost docs, (4) test title nit.