You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-ups flagged during PR #279/#280 review. Neither is a blocker for Phase 4 close but should land before Phase 4 is marked fully done.
1. `DecompositionContext.source_id → source_url` mapping is lossy
`kt_core_engine_api.decomposition.DecompositionContext.source_id` and the built-in `LlmDefaultFactDecompositionProvider` conflate "DB id" with "URL" — the adapter forwards `ctx.source_id` onto `TextExtractor`'s `source_url` kwarg because that's the closest analogue on the existing extractor.
Options:
Rename: `source_id → source_url` on the ABC. Matches existing extractor semantics; forces callers threading a bare DB id to be explicit.
Split: add both `source_id` and `source_url` typed fields. Providers pick whichever they need.
Prefer split — URL-less sources (uploaded files, future connectors) still have a DB id.
One PR per call site or one bundled — either works. Use caveman canary test runs to catch regression (`validate_all_graph_types` should stay quiet for the default graph type).
Follow-ups flagged during PR #279/#280 review. Neither is a blocker for Phase 4 close but should land before Phase 4 is marked fully done.
1. `DecompositionContext.source_id → source_url` mapping is lossy
`kt_core_engine_api.decomposition.DecompositionContext.source_id` and the built-in `LlmDefaultFactDecompositionProvider` conflate "DB id" with "URL" — the adapter forwards `ctx.source_id` onto `TextExtractor`'s `source_url` kwarg because that's the closest analogue on the existing extractor.
Options:
Prefer split — URL-less sources (uploaded files, future connectors) still have a DB id.
Files:
2. Remaining `DecompositionPipeline` consumer wire-ups
#280 wired only `decompose_chunk_task` (worker-search). Other call sites still run legacy `TextExtractor`:
Pattern per call site:
One PR per call site or one bundled — either works. Use caveman canary test runs to catch regression (`validate_all_graph_types` should stay quiet for the default graph type).
Related
No blockers. Both items are cleanup landed on Phase 4 foundations.