Skip to content

feat(search): add model-free lexical route for file corpus - #183

Closed
sun-970 wants to merge 2 commits into
bytefolk:mainfrom
sun-970:feature/176-file-lexical-lane
Closed

feat(search): add model-free lexical route for file corpus#183
sun-970 wants to merge 2 commits into
bytefolk:mainfrom
sun-970:feature/176-file-lexical-lane

Conversation

@sun-970

@sun-970 sun-970 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Closes rfc(search): is a model-free lexical lane intended for the file corpus? #176 (rfc: model-free lexical lane for the file corpus)
  • Adds route=lexical — FTS + trigram over files.name, no worker needed
  • Migration 0024: search_tsv tsvector + pg_trgm GIN on files (mirrors 0008 memory pattern)
  • search.Service.Search() bypasses worker check for RouteLexical
  • API accepts "lexical" in route validation; managed embedding path skipped (zero cost)
  • CLI: mem search "query" --route lexical
  • SPEC.md §6.2 + F5.4 extended; docs/mcp.md updated
  • Integration test proves lexical works with nil worker while text/auto still fail closed

Scope boundary (per #176)

  • No Chinese tokenizer change — to_tsvector('simple', ...) + pg_trgm, same as memories
  • No search ranking changes
  • No generation lifecycle changes

Validation ledger

  • Migration follows goose format, pg_trgm extension already installed by 0008
  • searchLexical reuses appendPathFilters, appendMIMEFilter, appendTimeFilters, scanHits — same filter guarantees as text/visual routes
  • Managed embedding executor bypass for lexical (no provider cost)
  • Integration test: 5 subtests covering lexical-without-worker, text/auto-still-fail, CJK trigram, path filter
  • SPEC.md, docs/mcp.md, CHANGELOG.md updated
  • CI pass (requires reviewer trigger)
  • Approval from non-author (required)

@liyuanyang
feat(search): add model-free lexical route for file search (bytefolk#176
e96bef2 
)

Files had no retrieval path without an embedding worker. Add a
`route=lexical` that uses FTS + trigram over files.name — the same
three-tier shape memory Recall already uses — so self-hosted
deployments that never configure a worker can still search filenames.

- Migration 0024: search_tsv tsvector + pg_trgm GIN on files
- search.Service: RouteLexical bypasses worker check, searchLexical
  with exact/FTS/trigram tiers
- API: accept "lexical" in route validation, skip managed embedding
  path (no cost)
- CLI: --route lexical
- SPEC.md §6.2, F5.4, docs/mcp.md: document the new lane
- Integration test proves lexical works with nil worker while
  text/auto still fail closed
@liyuanyang
refactor(search): drop unused reason column from lexical CTE
13ebe9e
PeterGuy326
PeterGuy326 requested changes Sep 9, 2026
View reviewed changes

@PeterGuy326 PeterGuy326 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate decision

REQUEST CHANGES on current head 13ebe9efa6ba3c733199d374e8db3d107f598ca2.

Blocking findings

  1. This PR adds server/internal/db/migrations/0024_files_lexical_search.sql, but open #180 also adds migration 0024_ann_hnsw_indexes.sql. Both cannot land with the same migration number; coordinate numbering and rebase one branch before merge.
  2. The PR body still records CI and approval as unchecked, and gh pr checks reports no checks for this head. Please run the repository CI against the exact head and attach the required non-author review evidence before requesting merge.

The focused local search/API packages pass, but that is not a substitute for the migration and full-pipeline gates.

sun-970 pushed a commit to sun-970/mem that referenced this pull request Sep 10, 2026
@liyuanyang
fix(index): scope to HNSW only, renumber migration to 0025
6cd0245 
- Remove benchmark/recall files (belong to bytefolk#184, not this PR)
- Rename migration 0024 → 0025 to resolve collision with bytefolk#183
- Fix verify script: use valid pgvector literals via array_fill,
  add index-scan assertion instead of just printing EXPLAIN output
- Update VALIDATION_HNSW.md references to 0025
@PeterGuy326

PeterGuy326 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Current-head CI approval readback (2026-09-10)

Head: 13ebe9efa6ba3c733199d374e8db3d107f598ca2. The original pending-approval runs on this head have been approved and real check-runs now exist (17 at this snapshot). No old-head run was approved. Auto-merge remains disabled; this is not a code approval or merge-ready declaration.

Failures at this snapshot:

This was referenced Sep 10, 2026
Merged
Merged
Closed
@sun-970

sun-970 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by @PeterGuy326's #194 which covers the same changes on current main. Closing per the '本轮先不合并' directive.

@sun-970 sun-970 closed this Sep 10, 2026
waterbro-8 pushed a commit that referenced this pull request Sep 17, 2026
@PeterGuy326
fix(search): publish lexical route corrections (successor to #183) (#194
6543983 
)

## Linked draft successor — original #183 remains open

Refs #176.

## Current follow-up: `c0421168bd145077bf164f91c0d2b454a79760ef`

- Fixed the remaining exported MCP route enum: `tools/list` now includes
`lexical`. The in-process MCP regression reproduced the missing enum
before the fix and now verifies both schema and `tools/call` HTTP
forwarding. All 9 MCP tests pass on Linux; affected-package vet passes.
- Added a strict, populated PostgreSQL sequential-upgrade regression and
a DB-free embedded-migration continuity guard. Actual PostgreSQL 16.14 /
pgvector 0.8.2 advances 23 → 24, verifies lexical backfill and full
history, and accepts ordinary production startup afterward. No
`AllowMissing` option or migration renumbering.
- Cumulative source/base order is #194 → #197 → #195. #197 remains HOLD
for text-query planner acceptance; #195 remains HOLD behind it. See
`docs/MIGRATION_SEQUENCE.md` for deployment and existing-gap recovery
boundaries.
- Tests ran at `2f2e965b6794863d7f5a38a748262da4691beb35`; this final
head only corrects the documented verification command to
`scripts/verify.sh integration`. Fresh exact-head CI and independent
review remain required. The earlier evidence below is historical, not a
fresh-head approval.

Preserves @sun-970 / liyuanyang's complete authored chain through
`13ebe9efa6ba3c733199d374e8db3d107f598ca2` for #176. This draft makes
the bounded reviewed corrections accessible; it does not replace
independent review, CI, or human approval. Do not close #183 before a
replacement is verified and merged.

The original fork's Git-data write returned HTTP 404 and its repository
permissions report `push:false`. No ACL override was attempted. This
canonical branch preserves original commit identities and hashes. Added
blobs, trees, and commits were checked against local Git hashes; no
force update was used.

## Corrections

- `scripts/verify.sh`: expect migration 24, matching this branch's
lexical migration. The former expectation of 23 caused the PostgreSQL
validation failure.
- Refresh audited Web development dependencies, traceable to #192 commit
`11e02e21ef2c3dbd2dae26e4376872e54e78ecb5` via a separate `cherry-pick
-x` commit. Audit threshold unchanged.
- No additional search/ranking changes beyond original #183.

## Validation at head `3d885a8427e06fd7175011ef0188a06f32028a3f`

- PASS: migration to 24 on PostgreSQL 16.14 / pgvector 0.8.2 and
PostgreSQL 17.10 / pgvector 0.8.3.
- PASS: `TestLexicalSearchWithoutWorker`, five cases: lexical without
worker, text/auto fail closed, CJK trigram, path restriction. Go tests
cross-compiled for Linux and executed against real disposable PostgreSQL
with `GOMAXPROCS=2` and serial package builds.
- PASS: audited lockfile check; shared fix from #192 remains explicit,
not attributed to this feature.
- NOT CLAIMED: provider-backed vector retrieval or production benchmark
quality.
- REQUIRED: fresh CI on this exact head and independent review.
Original-head CI/review is not approval of this draft.

Original feature scope: model-free file-corpus lexical route, migration
0024, API/CLI/docs, managed-provider bypass. Scope excludes tokenizer,
generation lifecycle, and ranking redesign.
waterbro-8 pushed a commit that referenced this pull request Sep 17, 2026
@PeterGuy326
fix(recall): publish fail-closed producer corrections (successor to #184
ae30bd5 
) (#196)

## Linked draft successor — original #184 remains open

Refs #175.

Preserves @sun-970 / liyuanyang's authored chain through
`89bb12bdf2ae5edbdfa17260953ec6352ab9239f` for #175. Do not close #184
before a replacement is independently verified and merged. This draft
remains HOLD for live acceptance.

Source-fork Git-data writes returned HTTP 404, with repository
`push:false`; no ACL override was attempted. The canonical branch
preserves original commit hashes and identities. Added blobs, trees, and
commits were individually hash-verified; no force update was used.

## Bounded corrections

- Fail closed on ambiguous cross-workspace mappings, unknown result
paths, malformed responses, non-finite scores, and failed requests;
retain an error artifact and exit 2.
- Map the shipping folder `path` plus file `name`; do not silently
discard unknown hits or infer tenant identity from snippets.
- Vector mode sends `route=text`; lexical mode sends `route=lexical`
with null provider/model/dimension metadata. Do not claim `auto` is
lexical/vector hybrid.
- Reject structured-memory queries this file-search endpoint cannot
serve. Document the existing file-only `profile-text-v1` fixture as the
bounded corpus.
- Remove hostname collection and invented provider/index identity.
Configuration labels remain explicitly operator-declared, not
server-verified.
- Carry #192 audit remediation as a separate `cherry-pick -x` of
`11e02e21ef2c3dbd2dae26e4376872e54e78ecb5`; audit threshold unchanged.

## Validation at `651bec1679c50a9cd07cf77b27b5556c20e3273e`

- PASS: Python 3.11.14, `python3.11 -m unittest discover -s
benchmarks/recall/tests`: 39 tests, including the reproduced fail-closed
regressions and a loopback HTTP fixture.
- PASS: `python3.11 -m benchmarks.recall verify`: deterministic harness
and intentional leakage failure gate.
- PASS: Web audit with the explicit shared fix.
- NOT VERIFIED: real memd retrieval, actual embedding-provider quality,
real index selection, production latency, or full structured-memory
corpus acceptance. The HTTP handler is a fixture, not memd; its timing
is not live benchmark evidence.

## Exact remaining live prerequisites (no external provider authorized)

1. An isolated, authorized test deployment of real memd and its Worker,
with PostgreSQL/pgvector and ingest dependencies configured, plus a
token verified to belong to the test workspace. No existing user
deployment or provider credentials have been used.
2. Ingest all five synthetic files from
`benchmarks/recall/data/profile-text-v1/corpus.jsonl`, preserving their
full paths and contents, into that workspace. The producer is not an
ingestor. Confirm indexing completed and file/result identities match
the fixture.
3. For the bounded fixed-text experiment, use the same locally
available, explicitly selected 768-dimensional text embedding model for
corpus and query. Verify the corpus/provider metadata and which active
generation or fixed table the server actually uses. A label passed to
the producer proves none of these facts. No paid provider, external
endpoint, model download, or provider configuration was enabled by this
correction.
4. Execute all four file queries through real `/v1/search` with `--mode
vector`, retain sanitized rankings, then score with `run --rankings` and
record the exact memd head, actual model/dimension/index, environment,
and errors. An empty/error run does not satisfy live acceptance.
5. Model-free lexical is a separate optional real-server experiment
requiring #183's server capability (draft successor #194); it cannot
establish vector quality. Full v1 structured-memory acceptance remains
unsupported by this producer and must not be reported as passed.

Fresh exact-head CI and independent review/human approval remain
separate required gates. No fake or paid live run is substituted for the
missing evidence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PeterGuy326 PeterGuy326 PeterGuy326 requested changes
@Bindy-lbb Bindy-lbb Awaiting requested review from Bindy-lbb Bindy-lbb is a code owner
@waterbro-8 waterbro-8 Awaiting requested review from waterbro-8 waterbro-8 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

rfc(search): is a model-free lexical lane intended for the file corpus?

3 participants

@sun-970 @PeterGuy326 @liyuanyang