refactor: lift DocAnchor to leaf module to break residual doc_linker ↔ store cycle (#501) - #502
Conversation
Reviewer's GuideRefactors doc-linker types and constants into a new leaf module to break the residual import cycle between aelfrice.doc_linker and aelfrice.store, while preserving the public doc_linker API and updating store to depend only on the new leaf module. Class diagram for DocAnchor and MemoryStore methods using itclassDiagram
class DocAnchor {
<<dataclass>>
+belief_id: str
+doc_uri: str
+anchor_type: str
+position_hint: str~None
+created_at: float
}
class MemoryStore {
+link_belief_to_document(belief_id: str, doc_uri: str, anchor_type: str, position_hint: str, created_at: float) DocAnchor
+get_doc_anchors(belief_id: str) list~DocAnchor~
+get_doc_anchors_batch(belief_ids: list~str~) dict~str, list~DocAnchor~~
}
MemoryStore ..> DocAnchor : returns
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since
doc_linker_typesis a leaf with no dependencies, you can likely move the function-local imports instoreto module scope (importingDocAnchor/ANCHOR_TYPESonce) to avoid repeated imports and make the dependency structure clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since `doc_linker_types` is a leaf with no dependencies, you can likely move the function-local imports in `store` to module scope (importing `DocAnchor` / `ANCHOR_TYPES` once) to avoid repeated imports and make the dependency structure clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
Move DocAnchor, ANCHOR_INGEST/MANUAL/DERIVED, ANCHOR_TYPES out of doc_linker into the new leaf module aelfrice.doc_linker_types. Drop the if-TYPE_CHECKING import of MemoryStore in doc_linker; the function signatures keep the "MemoryStore" forward-ref string, which `from __future__ import annotations` already makes a no-op at runtime. doc_linker re-exports the lifted symbols so external callers (`from aelfrice.doc_linker import DocAnchor`) keep working. Mirrors the leaf-module pattern PR #500 used for np_pattern, classification_core, and db_paths. No behavior change. Sets up the store.py edge swap in the next commit.
The three function-local imports in MemoryStore.link_belief_to_document, get_doc_anchors, and get_doc_anchors_batch now read from the leaf module aelfrice.doc_linker_types instead of aelfrice.doc_linker. Closes the residual edge that #494's merge introduced; the static cycle-finder reports 0 cycles ≤ length 4 across src/aelfrice now, matching the post-#500 invariant. Drops the inline comment about TYPE_CHECKING-only avoidance — the leaf module makes that workaround unnecessary; the comment's premise no longer holds. No behavior change. 2948 tests pass.
448c9c7 to
414f56c
Compare
|
[claim:review:Gylf:2026-05-08T20:57:14Z] |
|
[claim:review:Kulili:2026-05-08T20:57:26Z] |
|
[release:review:Kulili:2026-05-08T20:57:31Z] |
|
[release:review:Gylf:2026-05-08T20:57:57Z] |
Phase 1 of #501 — close the residual
doc_linker ↔ storecycle that #494's merge introduced and that PR #500's review flagged. Mirrors the leaf-module pattern PR #500 used fornp_pattern/classification_core/db_paths.Change
aelfrice.doc_linker_typesis a new leaf module containingDocAnchor,ANCHOR_INGEST/ANCHOR_MANUAL/ANCHOR_DERIVED, andANCHOR_TYPES. It imports nothing from the rest ofaelfrice— keep it that way.aelfrice.doc_linkerre-exports the lifted symbols so external callers (from aelfrice.doc_linker import DocAnchor) keep working. Theif TYPE_CHECKING: from aelfrice.store import MemoryStoreblock is gone —from __future__ import annotationsalready makes the"MemoryStore"forward-ref strings runtime no-ops.aelfrice.store's three function-local imports inlink_belief_to_document,get_doc_anchors, andget_doc_anchors_batchnow read fromdoc_linker_typesinstead ofdoc_linker.Static analysis
Cycle-finder from #499's body, run on the PR head:
doc_linker ↔ store).Both detectors agree at 0 now. Acceptance row for "cycle-finder reports 0 cycles ≤ length 4" met.
Test plan
tests/test_doc_linker.py+tests/test_retrieve_doc_anchors.py).notecount for module-import-cycle — verify on the next PR run picks up the cleanup.Refs
np_pattern/classification_core/db_paths)Closes #501.
Summary by Sourcery
Extract document linker anchor types into a leaf module to break the remaining import cycle between the doc linker and store while preserving the external API.
Enhancements: