chore(refactor): legacy post-Wave-1 cleanup (#45, #46, #47, #51)#61
Merged
Conversation
The langchain shim in src/bodhi_rag/indexing/ is residual
post-migration code from the bhodi -> bodhi-rag rename (Wave 1).
The production code path goes through the cross-context
ports (DocumentParserPort / ChunkerPort / EmbeddingPort /
VectorStorePort) wired by the composition root, so this
shim never executes in production.
Concretely, this removes:
- src/bodhi_rag/indexing/infrastructure.py (5 dead langchain
function-local imports; 93 lines)
- src/bodhi_rag/indexing/ports.py (legacy DocumentLoader /
DocumentSplitter / VectorStorePort Protocol, shadowed by
src/bodhi_rag/ports/)
- src/bodhi_rag/indexing/settings.py (IndexingSettings only
referenced inside the package itself; no external callers)
- tests/bodhi_rag/test_indexing_infrastructure.py (the
characterization test of the dead code)
The indexing bounded context __init__ is reduced to re-export
the two real use-case models (IndexDocumentsRequest /
IndexDocumentsResponse) and no longer claims IndexingSettings
or load_documents_from_{directory,file}.
The pyproject.toml ruff per-file override is tightened from
src/bodhi_rag/indexing/**/*.py to just
src/bodhi_rag/indexing/application/**/*.py because the only
remaining reason to allow function-local imports in the
indexing slice is to break circular dependency cycles inside
the application helpers (index.py, delete.py). The langchain
shim rationale no longer applies.
Closes #45
The interfaces package advertised tui and worker as public submodules via __all__, but the directories src/bodhi_rag/interfaces/tui/ and src/bodhi_rag/interfaces/worker/ were empty (only stale __pycache__/). No submodule existed, so from bodhi_rag.interfaces import tui would fail with ModuleNotFoundError, contradicting __all__. The empty directories are removed (filesystem hygiene; they were not tracked by git because the only contents were __pycache__/). __all__ is now restricted to the two real submodules: api and cli. This is the only post-PR-#39 cleanup needed in this scope; the src/bodhi_rag/AGENTS.md architecture diagram already reflected the api + cli reality. Closes #47
There were two distinct definitions of RetrievedDocument duplicating each other: - A Protocol in src/bodhi_rag/ports/vector_store.py with str, str, str, float, dict fields. - The actual dataclass in src/bodhi_rag/domain/entities.py using ChunkId and DocumentId value objects, score typed as float | None, and metadata typed as dict[str, Any]. Only the dataclass travelled through the system. The Protocol was only consumed as a TYPE_CHECKING import in four places: ports/reranker.py, retrieval/application/retrieve.py, answering/application/synthesize.py, and application/facade.py. This unifies the type: ports/vector_store.py keeps the VectorStorePort protocol but delegates the document shape to the domain dataclass. The four consumers import the dataclass from bodhi_rag.domain.entities. ports/llm.py does not import RetrievedDocument (it was migrated to message-oriented in PR #59), so it is unchanged. Result: a single source of truth for RetrievedDocument; strict type checkers (mypy --strict, pyright --strict) now resolve port signatures against the real shape; future renames of domain fields propagate through the ports without silent duck-type drift. Closes #51
src/bodhi_rag/application/facade.py carried dead code from a refactor regression: a private helper _merge_document_metadata that has no callers in the codebase, plus a RESERVED_PROVENANCE_KEYS frozenset that only _merge_document_metadata used. The canonical metadata-merge lives in src/bodhi_rag/indexing/application/index.py as _build_merged_document with its own _RESERVED_PROVENANCE_KEYS. Issue #46 originally reported two copies of _merge_document_metadata in facade.py; PR #59 already removed the duplicate, leaving a single copy behind. This commit removes that last copy together with the orphaned RESERVED_PROVENANCE_KEYS constant. A grep across src/, tests/, docs/, and README.md confirms zero references to either name remain in facade.py or anywhere else (the surviving _RESERVED_PROVENANCE_KEYS in src/bodhi_rag/indexing/application/index.py is the live canonical implementation, with a leading underscore and a different purpose). Closes #46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove dead code and consolidate types left behind by Wave 1
(bhodi -> bodhi-rag rename).
src/bodhi_rag/indexing/(~162 lines, never executed in production).
__all__lie insrc/bodhi_rag/interfaces/__init__.py(advertised
tuiandworkersubmodules that did not exist).RetrievedDocumenton the domain dataclass; deletethe duplicate Protocol in
ports/vector_store.py._merge_document_metadatahelper and orphanedRESERVED_PROVENANCE_KEYSconstant fromapplication/facade.py.Architecture
No behavioral changes. Pure code removal + type consolidation.
production code path goes through the cross-context ports
(DocumentParserPort / ChunkerPort / EmbeddingPort /
VectorStorePort) wired by the composition root.
__all__lie advertised non-existent submodules; acontributor trusting
__all__and writingfrom bodhi_rag.interfaces import tuiwould have hit animmediate
ImportError.RetrievedDocumentProtocol was a duck-typed shadow ofthe real domain dataclass. Ports now reference the dataclass
directly so strict type checkers (mypy --strict, pyright
--strict) resolve port signatures against the real shape.
_merge_document_metadatahelper was a refactorregression from commit 864c573; the canonical implementation
lives in
indexing/application/index.pyas_build_merged_document.Testing
Each commit was validated locally with
uv:uv run ruff check src tests-> all checks passeduv run mypy src/-> no issues found in 90 source filesuv run pytest-> 259 passed (2 tests removed were thecharacterization tests of the deleted dead code)
Total diff: 12 files changed, 12 insertions(+), 303 deletions(-).
Closes #45, #46, #47, #51