refactor(phase-5): core domain logic for retrieval, chunking, prompts

FORWARD_PORT_LOG.md

@@ -0,0 +1,27 @@
+# Forward Port Log
+
+Tracks `dev` changes during MODE 2 isolation (Phases 5-9).
+Each entry: what changed on `dev`, whether it was forward-ported or deferred
+to the cutover re-implementation queue.
+
+> Created retroactively at the start of Phase 5 (2026-04-29). The Phase 0-4
+> Mode 1 work merged from `dev` cleanly so this log starts empty.
+
+---
+
+## Forward-ported (critical)
+
+_Security fixes, data-loss bugs, production outages re-implemented against
+the new architecture. Each entry pairs the dev commit with the refactor
+commit so a reviewer can audit equivalence._
+
+(none yet)
+
+## Deferred to cutover (features)
+
+_Non-critical changes that landed on `dev` during MODE 2. These will be
+re-implemented directly in the new architecture during MODE 3 (Phases
+10-12) or post-cutover. List the dev PR number / commit and the target
+location in the new layout._
+
+(none yet)

REFACTORING_DECISION_LOG.md

@@ -55,7 +55,141 @@ match reality, then record the reasoning here.
 
 ---
 
-## Template for future entries
+## Phase 5 — Core domain logic for retrieval, chunking, prompts (2026-04-29)
+
+**1. New `RetrievalSearcher` port in `core/retrieval/searcher.py`, separate from
+the narrow `VectorStore` ABC.**
+The retriever needs four operations (search by query string, multi-query
+search, related-chunk lookup, ancestor lookup) that the Phase-4
+`VectorStore` ABC does not cover — that ABC is intentionally narrow
+(`search(embedding, top_k)`). We added a transitional ABC the retriever
+depends on, implemented by `services/storage/milvus_ray_shim.py` over the
+legacy Ray actor.
+- Why: STRATEGY §5A says the retriever should "call `VectorStore.search()`
+  (port method), not `vectordb.async_search.remote()`". But the legacy Ray
+  actor's `async_search` takes a query *string* and embeds internally; the
+  narrow `VectorStore.search(embedding, ...)` ABC doesn't fit. Pre-embedding
+  in the shim before calling Ray is impractical because the actor also owns
+  BM25 and surrounding-chunks semantics. A retrieval-facing port keeps the
+  retriever clean of Ray today and survives Phase 7 — when the Vectordb
+  god object is decomposed, these methods either move onto a richer
+  `VectorStore` or split between `VectorStore` and `ChunkRepository`.
+- Alternative considered: extend `VectorStore` with the four legacy methods.
+  Rejected — bloats the ABC with operations that should not exist past
+  Phase 7. Also considered: skip the new core port and have the retriever
+  call the Ray actor through the shim with the legacy method names —
+  rejected because that leaks legacy method names into core/ and makes the
+  retriever harder to test.
+
+**2. Skipped: bringing up integration tests for the new code.**
+Phase 5 ships pure-domain unit tests only (50 new tests in `core/`, no Ray /
+Milvus / real LLM). The new pipeline is dormant until Phase 8 wires it.
+- Why: Mode 2 forbids touching the legacy wiring; the new pipeline has
+  nowhere to be plugged in yet. Integration coverage will land with Phase 8
+  orchestrators (or Phase 7 storage if it goes first).
+- Alternative considered: stand up a fake searcher in an integration
+  fixture and run a full retriever-pipeline-RRF round trip. Defers the
+  same coverage to Phase 8 with less code; not worth the extra fixtures.
+
+**3. `Query`, `SearchQueries`, `TemporalPredicate` lifted into
+`core/models/query.py`.**
+The legacy `components/pipeline.py` defined these inline. The new
+`RetrieverPipeline` consumes them — they're domain types, not pipeline
+internals.
+- Why: STRATEGY §2 calls these out as `pipeline.py SearchQueries → core/models/query.py`.
+- Alternative considered: keep them in `core/retrieval/`. Rejected — they
+  describe a query in the abstract; the orchestrator (Phase 8) and the API
+  layer will both use them, not just retrieval.
+
+**4. Phase 5.15 (re-export shims) deferred to follow-up — then completed.**
+The first Phase 5 commits (5A/5B/5C, 2026-04-29) created the new core/
+modules but left the legacy `components/` files intact. STRATEGY §4.1
+mandates a three-step move — create new file, update old file to
+re-export from new, update consumers — and Phase 5 step 5.15 says
+"Update old files to re-export from core/". We skipped that.
+The follow-up sweep (2026-05-05) replaced six legacy files with shims:
+
+| Legacy file | Shim strategy |
+|---|---|
+| `components/indexer/chunker/utils.py` | Plain re-export from `core.chunking.markdown_utils` |
+| `components/prompts/prompts.py` | `load_prompt(key)` adapter calling `core.prompts.template_loader.load_template_by_key` |
+| `components/utils.py:format_context` + `format_web_context` | Adapters into `core.prompts.chat_prompt_builder` (rest of utils stays — Phase 6+ scope) |
+| `components/indexer/chunker/chunker.py` | `BaseChunker` / `RecursiveSplitter` delegate to `core.chunking.RecursiveSplitter` via Document↔ProcessedDocument↔Chunk conversion. `ChunkContextualizer` + `ChunkerFactory` retained (5D + Phase 8). |
+| `components/retriever.py` | `Single`/`MultiQuery`/`HyDe` retrievers wrap `core.retrieval.retriever` strategies. Ray actor → `MilvusRayShim`; `ChatOpenAI` → `_LangChainLLMAdapter`. `RetrieverFactory` retained. |
+| `components/pipeline.py` | `Query`/`SearchQueries`/`TemporalPredicate` re-exported from `core.models.query`. `RetrieverPipeline` delegates to `core.retrieval.pipeline.RetrieverPipeline` via a `_LegacyRerankerAdapter` bridging the legacy reranker (Document-in / Document-out) to the core ABC (str-in / `(idx, score)`-out). `RagPipeline` + `RAGMODE` retained (Phase 8). |
+
+- Why: STRATEGY §4.1 is explicit ("Update old file to re-export from new
+  location"); leaving the duplication in place would let the codepaths
+  drift. Three CodeRabbit fixes from PR #352 (image_caption ChunkType,
+  page-marker semantics, chunk_table header-only flush) had to be applied
+  twice or only fixed in core — exactly the failure mode 5.15 prevents.
+  The shim pattern matches the prior-art shims for config (1329cc18) and
+  exceptions (a0f3d9f2).
+- Alternative considered: leave the copies until Phase 8 cutover. Rejected
+  — the doc explicitly puts 5.15 *inside* Phase 5, and one round of
+  drift already happened.
+- Side effect: a noqa side-effect import in `chunker.py` keeps the legacy
+  `components.utils` ↔ `components.indexer.utils.files` circular-import
+  resolving in the right order. Removed once `components.utils` is split
+  in Phase 6+.
+- Behavioral note: image elements in the legacy chunker now stamp
+  `chunk_type=image_caption` (matching `core.models.chunk.ChunkType`)
+  instead of the previous raw `image`. No legacy reader filters on this
+  value, so the change is invisible to consumers.
+
+**5. `core/chunking/recursive.py` keeps `langchain.text_splitter.RecursiveCharacterTextSplitter` as a dependency.**
+STRATEGY §3 lists core as "stdlib + pydantic + pure libs" and §4.7 limits
+LangChain in core to boundary converters (`from_langchain` / `to_langchain`
+on domain models). The chunker imports `RecursiveCharacterTextSplitter`
+directly, which is neither stdlib nor a boundary converter.
+- Why: `RecursiveCharacterTextSplitter` is a self-contained recursive
+  separator-based string splitter — no IO, no LLM client, no Document
+  semantics. Reimplementing it in core/ would be a meaningful chunk of
+  pure code with no behavior change, and the legacy chunker has been
+  using it for two years with stable output. Keeping it in for Phase 5
+  preserves byte-for-byte chunk equivalence, which the strangler-fig
+  shim relies on for behavior parity. The import is also deferred
+  (inside the constructor / `split_text`) so importing the module
+  without LangChain installed doesn't fail.
+- Alternative considered: write a stdlib-only recursive splitter as part
+  of Phase 5. Rejected as scope creep — would couple a behaviorally
+  risky rewrite (chunk boundaries shift, downstream embedding output
+  changes) to the additive Phase 5 cut, breaking the parity guarantee
+  the legacy shims rely on. Tracked as a Phase 12 / post-cutover
+  follow-up: replace the splitter with a stdlib implementation behind
+  the same `Callable[[str], int]` length-function injection point.
+- Scope: limited to `RecursiveCharacterTextSplitter`. No other LangChain
+  symbol leaks into core; `langchain_core.documents.Document` only
+  appears inside `Chunk.from_langchain` / `Chunk.to_langchain` /
+  `Document.from_langchain` / `Document.to_langchain`, all with deferred
+  imports, exactly as §4.7 prescribes.
+
+**6. Two file-layout deviations from STRATEGY §3 / §5A / §5B.**
+- `core/chunking/markdown_section.py` (§5B, line 1038; §3, line 387)
+  → renamed to `core/chunking/markdown_utils.py`. The contents are pure
+  parsing helpers — `MDElement`, `split_md_elements`, `chunk_table`,
+  `parse_markdown_table`, `get_chunk_page_number` — not a
+  section-aware chunker strategy. The name `markdown_utils.py` matches
+  the module's role (utilities consumed by `RecursiveSplitter`); the
+  separate `markdown_section.py` / `markdown_layout.py` *strategies*
+  listed in §3's tree aren't built in Phase 5 and remain available
+  filenames if/when those strategies land.
+- `core/retrieval/hydration.py` (§5A, line 1027) → kept as private
+  `_expand_with_related_chunks` in `retriever.py`. The function is
+  ~60 LOC, only invoked by `BaseRetriever.expand_search_results`, and
+  splitting it would add an import + test fixture surface without any
+  reuse benefit. If a second consumer ever appears (Phase 8 likely),
+  promoting it to a module-level public function in `hydration.py` is
+  a one-commit move.
+- Why: both deviations make the module names track the actual contents
+  rather than the strategy doc's pre-write naming guess. Recording so
+  future readers don't grep for files that aren't there.
+- Alternative considered: rename to match the strategy doc verbatim.
+  Rejected — the strategy filenames anticipated different content
+  (a section/layout chunker, a standalone hydration entry point) than
+  what Phase 5 actually produced.
+
+---
 
 ## Phase 1 — Registry + Exceptions (2026-04-21)