Skip to content
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions FORWARD_PORT_LOG.md
Original file line number Diff line number Diff line change
@@ -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)
84 changes: 83 additions & 1 deletion REFACTORING_DECISION_LOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,89 @@ 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.

---

## Phase 1 — Registry + Exceptions (2026-04-21)

Expand Down
Loading
Loading