# Modular RAG Pipeline Decisions ## Discover Summary - Capability source: sm-flow Discover using local repository evidence and existing OpenSpec/devflow context. - Slug: `modular-rag-pipeline`. - Scale: standard. - Goal: turn `lookup_knowledge` into a modular RAG pipeline suitable for Agent engineering interview use, while keeping the explicit Agent tool and evidence trace. ## Context Evidence ### Existing Architecture - `mvp/architecture/rag-architecture.md` documents the desired boundary: mature framework retrieval plus business-observable orchestration. - `mvp/architecture/retrieval-observability.md` says L0 is a hint/explainability layer and L1 semantic retrieval is the default recall path. - `VectorSearchService` is already the retrieval facade and supports Spring AI `VectorStore` with SDK fallback. - `LookupKnowledgeTool` currently still owns query analysis, L1 invocation, relevance normalization, result assembly, evidence block construction, session dedup, and recorder calls. ### Existing Evidence Blocks - `EvidenceBlock` already exists. - `LookupResult` already has `evidenceBlocks`, `evidenceCandidateCount`, and `evidenceBlockCount`. - `LookupKnowledgeTool` currently builds evidence blocks internally. - `ToolInvocationRecorder` already persists compact evidence block summaries in `retrieval_details`. Conclusion: evidence blocks are partially implemented, but the post-retrieval module boundary is not. ### Current Gaps - Context packing is not a first-class module. - Rerank is not a first-class module; only original semantic rank and trace summary sorting exist. - The `primary` / `supplement` result model still encodes old L0/L1 semantics. - `lookup_knowledge` tool description still describes the old two-stage retrieval model. ## Question Pool | ID | Dimension | Question | Mode | Status | | --- | --- | --- | --- | --- | | Q1 | Boundary | Should this be internal-only refactor or allow return contract changes? | user-interview | confirmed | | Q2 | Fallback | If filtered L1 fails, should L0 provide weak fallback evidence? | user-interview | confirmed | | Q3 | Interface | Can `LookupResult` add new fields and remove old `primary` / `supplement` if simpler? | user-interview | confirmed | | Q4 | Architecture | Does current repo already have evidence blocks, context packing, and rerank? | evidence-driven | resolved | | Q5 | Compatibility | Which in-repo consumers reference `primary` / `supplement`? | evidence-driven | resolved | | Q6 | Commit detail | What are the exact fields and thresholds for traces/context pack/low quality? | user-interview or specify | pending | ## Confirmed User Decisions ### D1: Prefer one-shot modular RAG refactor User confirmed that the change can be done "一次到位" instead of only doing a compatibility-preserving internal refactor. Implementation implication: - Create full pipeline modules now. - Do not leave `LookupKnowledgeTool` as a large procedural class. ### D2: L0 is not a normal evidence retrieval path User challenged the first design because it made L0 participate in too many flows. Confirmed direction: ```text L0 -> query understanding / category filter / domain/entity/keyword signal L1 -> main vector retrieval ``` Implementation implication: - Do not model L0 and L1 as equal retrievers in the normal path. - L0 output can influence filter, rerank, and trace. ### D3: MVP fallback is unfiltered L1 retry User proposed a simpler MVP fallback: ```text Filtered L1 using L0 category filter -> if inaccurate or empty -> retry raw query with L1 and no L0 filter ``` Confirmed direction: - Use filtered vector retrieval first when L0 provides an unambiguous category. - If filtered retrieval is low-quality, retry unfiltered vector retrieval with raw query. - Do not return L0 documents as fact evidence fallback in this MVP design. ### D4: Result contract can change User confirmed new fields can be added and old fields can be removed if the later flow becomes cleaner. Implementation implication: - `LookupResult.primary` and `LookupResult.supplement` may be removed. - Preferred contract becomes `evidenceBlocks + contextPack + retrievalTrace + rerankTrace`. - This is a breaking interface change and must be treated as L4. ## Evidence-Driven Findings ### E1: Existing spec conflict `openspec/specs/rag-knowledge-retrieval/spec.md` currently says L1 no-result or failure should return an L0-based primary result. This conflicts with the confirmed design. Required OpenSpec update: - Replace L0 primary fallback with unfiltered vector retry. - Define no-evidence behavior when both filtered and unfiltered L1 fail. ### E2: Existing compatibility-field requirement conflict `openspec/specs/rag-knowledge-retrieval/spec.md` currently requires `primary` and `supplement` compatibility fields to remain when evidence blocks exist. Required OpenSpec update: - Remove compatibility-field requirement. - Define evidence blocks and context pack as the preferred tool result contract. ### E3: Primary/supplement references are localized Search found concrete Java references in: - `LookupKnowledgeTool` - `ToolInvocationRecorder` - `LookupKnowledgeToolTest` - `ToolInvocationRecorderTest` - `LookupResult` - `PrimaryResult` - `SupplementResult` No broad in-repo service usage was found beyond tool implementation, recorder, tests, prompts, and historical docs. Implementation implication: - One-shot migration is feasible if tests and prompts are updated in the same change. ### E4: Existing trace contract must be preserved `tool_invocation` is used by diagnosis trace, verifier, and evaluation code. The database table does not need to change for this design if new details remain inside `retrieval_details`. Implementation implication: - Keep table-level fields stable. - Enrich JSON `retrieval_details` with `retrieval_trace`, `rerank_trace`, `context_pack_summary`, and fallback reason. ## Interface Impact Level: L4 breaking interface. Reason: - Removes or changes old result fields consumed by current tests and possibly by Agent prompt behavior. - Changes `lookup_knowledge` tool JSON shape. Mitigation: - Keep tool name and input signature unchanged. - Update all in-repo consumers in the same change. - Keep `tool_invocation` table schema stable. - Add tests for the new result contract. - Update prompt text to teach Agent to use `contextPack` and `evidenceBlocks`. ## Proposed Implementation Shape Pipeline classes: - `KnowledgeQueryTransformer` - `KnowledgeDocumentRetriever` - `KnowledgeEvidencePostProcessor` - `KnowledgeContextPacker` - `LookupResultAssembler` New or updated DTOs: - `KnowledgeQuery` - `RetrievedEvidenceCandidate` - `EvidencePostprocessResult` - `ContextPack` - `RetrievalTrace` - `RerankTrace` - `LookupResult` Policy: - L0-derived filter is optional and only used when unambiguous. - Filtered L1 low-quality result triggers raw unfiltered L1 retry. - Rule-based rerank is sufficient for MVP. - Context packing uses character budget first, not exact token counting. ## Pending For Commit - Specify exact fields for `ContextPack`, `RetrievalTrace`, and `RerankTrace`. - Specify low-quality trigger for unfiltered retry. - Decide whether `PrimaryResult` / `SupplementResult` classes are deleted or deprecated during the first apply. - Write OpenSpec `design.md`, specs, and executable `tasks.md`. ## Specify Results Created committed-design artifacts: - `design.md` - `specs/rag-knowledge-retrieval/spec.md` - `tasks.md` Resolved pending items: - `ContextPack` minimum fields: `packedText`, `strategy`, `charBudget`, `usedChars`, `includedSources`, `omittedSources`. - `RetrievalTrace` minimum behavior: record filtered attempt, unfiltered retry when used, fallback reason, and no-evidence paths. - `RerankTrace` minimum behavior: record final rank, source, base retrieval score when available, and major boost reasons for top evidence blocks. - Low-quality trigger: empty candidates, empty final evidence, or top normalized similarity below `retrieval.normalization.reference-threshold`. - `PrimaryResult` / `SupplementResult`: may be removed during apply if all compile-time usages are migrated. ## Cross-Artifact Alignment | Check | Result | | --- | --- | | proposal goals/scope -> design decisions | aligned | | design module boundaries -> specs behavior | aligned | | specs observable behavior -> tasks | aligned | | interface impact -> design/tasks migration work | aligned | No cross-artifact gaps remain for Commit. ## Architecture Audit Input -> processing -> output chain: ```text lookup_knowledge(query) -> KnowledgeQueryTransformer -> KnowledgeDocumentRetriever -> KnowledgeEvidencePostProcessor -> KnowledgeContextPacker -> LookupResultAssembler -> ToolInvocationRecorder ``` Risk assessment: - The architecture keeps the explicit Agent tool boundary and does not move retrieval into an implicit Advisor. - Data ownership is clearer: query hints belong to transformer, vector candidates to retriever, evidence/context/traces to post-retrieval pipeline, persistence summaries to recorder. - The largest risk is the L4 result contract change; design and tasks require prompt/test/recorder migration in the same apply. - Database migration risk is low because `tool_invocation` table fields remain stable and new trace details stay in JSON. - Latency risk from unfiltered retry is accepted for MVP because retry only happens below reference quality. ## Commit Gate OpenSpec validation: ```text openspec validate modular-rag-pipeline --strict Change 'modular-rag-pipeline' is valid ``` File integrity: - proposal exists and states problem, proposed change, scope, non-goals, risks, and interface impact. - design exists and records module boundaries, decisions, migration, rollback, and risks. - specs exist and define observable behavior for modular pipeline, unfiltered retry, evidence-first result, context pack, rerank, and L0 hint boundaries. - tasks exist and are executable vertical slices. Consistency: - proposal core concepts are represented in design. - design decisions are represented in specs and tasks. - tasks have verifiable implementation and test steps. - L4 interface impact is recorded and mapped to migration tasks. Status: ready to mark as Committed OpenSpec. ## Pre-apply Research Capability source: `openspec-apply-change` + sm-flow apply protocol. `codebase-retrieval` and LSP tools were not available in this session, so call-chain confirmation used OpenSpec context, `rg`, targeted file reads, and tests. Reference implementation and affected files inspected: - `src/main/java/com/superbiz/agent/tool/LookupKnowledgeTool.java` - `src/main/java/com/superbiz/agent/service/KnowledgeIndexService.java` - `src/main/java/com/superbiz/agent/service/VectorSearchService.java` - `src/main/java/com/superbiz/agent/tool/RetrievedDocTracker.java` - `src/main/java/com/superbiz/agent/service/ToolInvocationRecorder.java` - `src/main/java/com/superbiz/agent/dto/LookupResult.java` - `src/test/java/com/superbiz/agent/tool/LookupKnowledgeToolTest.java` - `src/test/java/com/superbiz/agent/service/ToolInvocationRecorderTest.java` - `src/main/resources/prompts/chat-executor-prompt.md` - `src/main/resources/prompts/executor-prompt.md` - `src/main/resources/application.yml` Technical stack checklist: - Request/response structure: `lookup_knowledge` returns a Java DTO serialized as Agent tool JSON. - Retrieval facade: `VectorSearchService.searchSimilarDocuments(query, topK, category)` is the stable retrieval API and already hides Spring AI VectorStore vs SDK fallback. - L0 query hints: `KnowledgeIndexService.analyzeQuery` returns `L0Hint(matches, matchedKeywords, domains, entities, titles)` and `singleDomainOrNull()`. - Session dedup: `RetrievedDocTracker` stores `sessionId -> domain -> filePath` and exposes backward-compatible `isAlreadyRetrieved`. - Trace persistence: `ToolInvocationRecorder.recordLookupKnowledge` writes stable table columns and JSON `retrieval_details`. - Prompt consumers: executor prompts still describe old L0/L1 and `primary.content`; these must be migrated. - Tests: `LookupKnowledgeToolTest` is the main consumer of old `primary` / `supplement` assertions; recorder tests verify retrieval details. Implementation decision: - Add pipeline DTOs under `com.superbiz.agent.dto`. - Add pipeline services under `com.superbiz.agent.service`. - Keep `VectorSearchService` unchanged. - Keep `lookup_knowledge` tool name and query argument unchanged. - Delete `PrimaryResult` / `SupplementResult` only after production and tests stop referencing them. ## Apply Results Completed tasks: 31/31. Implemented: - Added modular RAG DTOs: `KnowledgeQuery`, `RetrievedEvidenceCandidate`, `ContextPack`, `RetrievalTrace`, `RerankTrace`, `EvidencePostprocessResult`. - Added pipeline services: `KnowledgeQueryTransformer`, `KnowledgeDocumentRetriever`, `KnowledgeEvidencePostProcessor`, `KnowledgeContextPacker`, `LookupResultAssembler`. - Refactored `LookupKnowledgeTool` into a thin orchestrator. - Migrated `LookupResult` to evidence-first fields and removed `primary` / `supplement`. - Deleted `PrimaryResult` and `SupplementResult`. - Updated `ToolInvocationRecorder` to persist query transform, retrieval trace, context pack summary, rerank trace, fallback reason, and evidence summaries in `retrieval_details`. - Updated executor prompt text for `contextPack` / `evidenceBlocks`. - Updated RAG architecture docs. - Rewrote lookup and recorder tests for the new contract. Verification: ```text mvn -q -DskipTests compile PASS mvn -q "-Dtest=LookupKnowledgeToolTest,ToolInvocationRecorderTest" test PASS openspec validate modular-rag-pipeline --strict PASS: Change 'modular-rag-pipeline' is valid ``` Full suite attempt: ```text mvn -q test FAIL ``` The full suite failed on pre-existing/environment-dependent tests: - `MilvusConnectionTest.connect`: `MILVUS_TOKEN` not set. - `RedisSessionManagerTest`: Redis JSON contains legacy `messagePairCount`, not accepted by current `SessionContext`. - Spring context / repository tests attempted MySQL/Flyway and failed when database connectivity was unavailable in the first sandboxed run. The full suite was retried outside the sandbox after approval. It still failed for the Milvus token and Redis serialization issues above, so these failures are not attributed to the modular RAG change. Diff scope reviewed: - RAG implementation: DTOs, pipeline services, `LookupKnowledgeTool`, `ToolInvocationRecorder`. - Contract cleanup: removed `PrimaryResult` / `SupplementResult`, updated `LookupResult`. - Tests: lookup tool and recorder tests. - Prompts: executor and chat executor guidance. - Docs/OpenSpec: modular RAG change files and architecture docs. Known remaining risk: - Runtime Agent prompt behavior should be demo-tested manually because the tool JSON contract changed from `primary/supplement` to `evidenceBlocks/contextPack/traces`. ## Review Results Review finding: - Session dedup returned `found=false` but still carried `evidenceBlocks` and `contextPack`, allowing the Agent to consume duplicate evidence despite the dedup message. Fix: - `LookupResultAssembler.deduped` now returns an empty evidence/context payload while preserving trace, relevance hint, and retrieved-domain memory. - Added `LookupKnowledgeToolTest.sessionDedupDoesNotReturnConsumableEvidenceAgain`. Additional verification: ```text mvn -q -DskipTests compile PASS mvn -q "-Dtest=LookupKnowledgeToolTest,ToolInvocationRecorderTest" test PASS MILVUS_TOKEN= mvn -q test PASS openspec validate modular-rag-pipeline --strict PASS: Change 'modular-rag-pipeline' is valid git diff --check PASS with LF/CRLF warnings only ``` Updated full-suite note: - `MilvusConnectionTest` passes when `MILVUS_TOKEN` is injected into the Maven process from `application.yml`. - `RedisSessionManagerTest` now passes after marking computed `SessionContext` getters as non-serialized JSON properties and ignoring unknown legacy Redis fields.