Skip to content

refactor(retrieval): freeze the v1.5 retrieval, ingestion and citation seams - #161

Merged
mrsibe merged 3 commits into
mainfrom
refactor/160-v15-seams
Sep 28, 2026
Merged

mrsibe merged 3 commits into
mainfrom
refactor/160-v15-seams

Conversation

@mrsibe

@mrsibe mrsibe commented Sep 28, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

Introduces the retrieval, ingestion and citation contracts that v1.5 builds on, with no user-visible behavior change.

Why?

The three v1.5 workstreams all need the same seams, and none existed:

Trust      #155 → #156 → #157      (#94 → #157)
Source     #95 → #98 → #158
Retrieval  #95 → #78 → #96 → #77

Without this step, each issue would grow its own version of retrieval, ingestion and citation, and KnowledgeService would absorb all of them. This is step 0 of epic #154.

Related issue

Fixes #160
Related to #154

What changed

  • Retrieval contract — Retriever.search(request: RetrievalRequest): Promise<RetrievalResult>; RetrievalResult.trace records strategy, scope, topK, threshold, durationMs via a pure buildRetrievalTrace().
  • KnowledgeService.retrieve() is the new entry point and keeps the embedding-space precondition. The legacy search() keeps its SearchResult[] shape and now delegates to retrieve(), so existing callers are unchanged.
  • A requested source filter is refused, not ignored (assertFilterSupported) until [Feat] Source scope: retrieve from the current or selected sources, persisted per session #94 implements pre-KNN filtering. The dead VectorStore.QueryOptions.filter — declared but never read, with no matching clause in knnQuerySql() — is removed. Declaring a capability that does nothing is what this issue exists to stop.
  • CitationCandidate declared in shared/types/citation.ts for [Feat] Exact-passage citation: the model cites source spans, not chunks #155: the cited span exists before the answer, so no post-hoc semantic alignment is needed.
  • Ingestion run/state contract in src/main/services/ingestion/types.ts: IngestionStage, IngestionRun, the SourceState / IndexState split, and isTerminalStage(), for [Feat] Ingestion runs: persistent pipeline state, retry, and non-destructive re-index #95.

How was this tested?

  • npm run typecheck — passes (node, web, test).
  • npm test — 345 pass, 0 fail (5 new: test/retrievalContract.test.ts, test/ingestionTypes.test.ts).
  • npm run build — passes.
  • Eval baseline acceptance: ran the RAG harness against a temp output dir —
    electron . --eval-harness --eval-out=/tmp/eval-out — and the deterministic report is byte-for-byte identical to docs/eval/baseline-v1.4.json (diff clean). docs/eval/ in the repo was not modified.
  • Not run: npm run smoke:packaged (no change to Electron/packaging/db-init; the only new module is a pure types entry under src/main/services/ingestion/).

Checklist

  • I have reviewed my own changes.
  • npm run typecheck passes.
  • npm run build passes.
  • I have tested the affected user workflow. (Retrieval path unchanged; eval metrics identical.)
  • I have not included unrelated changes.
  • I have updated documentation when necessary. (None required; contracts carry their rationale inline.)

Desktop / build changes

  • Not applicable

…n seams

Introduce the contracts v1.5 builds on, with no user-visible behavior change.

- Retriever.search() takes a RetrievalRequest and returns a RetrievalResult
  carrying a RetrievalTrace (strategy, scope, topK, threshold, durationMs).
  KnowledgeService gains retrieve() and keeps search()'s legacy shape as a
  delegate over it.
- A requested source filter is refused until #94 implements pre-KNN filtering,
  instead of being silently ignored. Remove the dead, never-read
  VectorStore.QueryOptions.filter, which declared a capability that did nothing.
- Declare CitationCandidate for #155: the cited span exists before the answer,
  so no post-hoc (and non-deterministic) semantic alignment is needed.
- Declare ingestion run/stage and the source/index state split for #95.

Verified: npm run typecheck; npm test (345 pass); the RAG eval harness
reproduces docs/eval/baseline-v1.4.json byte-for-byte.
@mrsibe mrsibe added refactor Internal restructuring, no behavior change area:foundation Module boundaries, architecture, tech debt area:retrieval Retrieval quality and evaluation labels Sep 28, 2026
…splay id

The candidate's identity in the answer is the same [n] the model writes and
Citation.index carries; a string id like 'S2' would put a UI format into the
domain model. blockId is optional because not every source has block structure.
…no blocks

A chunk indexed before block provenance, or from a source that produces no
blocks, has no span to point at. The fallback keeps the whole-chunk quote, so
the offsets must be optional rather than a fabricated 0..0.
@mrsibe
mrsibe merged commit 2539b5a into main Sep 28, 2026
3 checks passed
@mrsibe
mrsibe deleted the refactor/160-v15-seams branch September 28, 2026 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:foundation Module boundaries, architecture, tech debt area:retrieval Retrieval quality and evaluation refactor Internal restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactor] v1.5 architecture prep: freeze the retrieval, ingestion and citation seams

1 participant