docs(feature-intentional-clustering): spec memo for #436 - #455
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new specification document for Intentional Clustering in retrieval has been added, detailing the design of a two-stage API (cluster_candidates and pack_with_clusters), clustering criteria based on graph-connected components with weight thresholds, a diversity-aware greedy packing algorithm, configuration parameters, latency expectations, and reconciliation with existing components. No code implementation is included. ChangesIntentional Clustering Specification
Possibly Related Issues
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~30 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Reviewer's GuideDocs-only spec memo that defines the intentional clustering feature (#436): it formalizes a graph-connected clustering definition, a two-stage diversity-aware pack algorithm, configuration and defaults, latency/bench expectations, and how this retrieval-time transform composes with existing features and test/bench gating. Sequence diagram for retrieval pipeline with intentional clusteringsequenceDiagram
actor User
participant RetrieveAPI as RetrieveAPI
participant RetrievalEngine as RetrievalEngine
participant Store as Store
User->>RetrieveAPI: retrieve(query, use_intentional_clustering)
RetrieveAPI->>RetrievalEngine: lane_fan_out(query)
RetrievalEngine-->>RetrievalEngine: BM25F + heat_kernel + HRR_structural + BFS
RetrievalEngine-->>RetrievalEngine: score_composition_and_rank
alt use_intentional_clustering is true
RetrievalEngine->>Store: edges_for_beliefs(candidate_ids)
Store-->>RetrievalEngine: edges
RetrievalEngine-->>RetrievalEngine: cluster_candidates(candidates, candidate_scores, edges)
RetrievalEngine-->>RetrievalEngine: pack_with_clusters(clusters, token_budget)
RetrievalEngine-->>RetrieveAPI: packed_beliefs_with_clustering
else use_intentional_clustering is false
RetrievalEngine-->>RetrievalEngine: existing_pack_loop(candidates, token_budget)
RetrievalEngine-->>RetrieveAPI: packed_beliefs_without_clustering
end
RetrieveAPI-->>User: RetrievalResult
Class diagram for RetrievalCluster and clustering contractclassDiagram
class RetrievalCluster {
+int cluster_id
+tuple~str~ member_ids
+str representative_id
+float seed_score
}
class Belief {
+str id
+float score
+int token_cost
}
class Edge {
+str src_id
+str dst_id
+float weight
+str edge_type
}
class ClusterModule {
+list~RetrievalCluster~ cluster_candidates(candidates, candidate_scores, edges, edge_weight_floor)
+list~Belief~ pack_with_clusters(clusters, token_budget, cluster_diversity_target, fallback_to_score)
}
class UnionFind {
+make_set(node)
+find(node)
+union(node_a, node_b)
}
class Config {
+float DEFAULT_CLUSTER_EDGE_FLOOR
+int DEFAULT_CLUSTER_DIVERSITY_TARGET
+bool use_intentional_clustering
+float cluster_edge_weight_floor
+int cluster_diversity_target
}
Belief "1" <-- "*" RetrievalCluster : member_ids
RetrievalCluster "1" --> "1" Belief : representative_id
ClusterModule --> RetrievalCluster : produces
ClusterModule --> Belief : returns
ClusterModule --> Edge : consumes
ClusterModule --> UnionFind : uses
ClusterModule --> Config : reads
UnionFind <.. DedupDuplicateCluster : reused_pattern
class DedupDuplicateCluster
Flow diagram for two stage diversity aware pack algorithmflowchart TD
A[Start pack_with_clusters] --> B[Init out list, used_tokens, covered_clusters]
B --> C[Stage 1 iterate clusters sorted by descending seed_score]
C --> D{covered_clusters size >= cluster_diversity_target?}
D -- Yes --> H[Proceed to Stage 2]
D -- No --> E[Select cluster representative rep]
E --> F{rep token_cost fits within token_budget?}
F -- Yes --> G[Append rep to out, update used_tokens and covered_clusters]
G --> C
F -- No --> I{fallback_to_score is true?}
I -- Yes --> H
I -- No --> C
H[Stage 2] --> J[Build score_ranked_remaining from clusters excluding already selected beliefs]
J --> K[Iterate beliefs in score_ranked_remaining]
K --> L{belief token_cost fits within remaining token_budget?}
L -- Yes --> M[Append belief to out and update used_tokens]
M --> K
L -- No --> N[Break Stage 2 loop]
N --> O[Return out]
K -->|no more beliefs| O[Return out]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The interaction between
pack_with_clustersand existing locked-belief handling is only briefly mentioned; consider explicitly specifying the exact pre-inclusion step and order of operations so the implementation can mirror current L0 semantics without guesswork. - The
edge_weight_floorandcluster_diversity_targetdefaults are justified qualitatively, but you might want to spell out expected failure modes (e.g., over- or under-clustering) to guide future tuning and avoid misinterpreting bench results when these knobs are adjusted. - Stage 2 references
flattened_score_order(clusters)andcandidate_scoreswithout fully defining that ordering in the contract; it would help to explicitly define how ties, cluster membership, and any secondary keys are handled so multiple implementations converge on the same behaviour.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The interaction between `pack_with_clusters` and existing locked-belief handling is only briefly mentioned; consider explicitly specifying the exact pre-inclusion step and order of operations so the implementation can mirror current L0 semantics without guesswork.
- The `edge_weight_floor` and `cluster_diversity_target` defaults are justified qualitatively, but you might want to spell out expected failure modes (e.g., over- or under-clustering) to guide future tuning and avoid misinterpreting bench results when these knobs are adjusted.
- Stage 2 references `flattened_score_order(clusters)` and `candidate_scores` without fully defining that ordering in the contract; it would help to explicitly define how ties, cluster membership, and any secondary keys are handled so multiple implementations converge on the same behaviour.
## Individual Comments
### Comment 1
<location path="docs/feature-intentional-clustering.md" line_range="165" />
<code_context>
+
+Issue acceptance #4: *"Cluster pass runs inside the retrieve() budget; no separate query."* The cost decomposes into:
+
+- **One `store.edges_for_beliefs(candidate_ids)` call.** Indexed lookup; existing pattern from BFS multi-hop. Cost is dominated by the `IN (...)` clause; at typical candidate-pool size (≤200) this is sub-ms in SQLite-backed numpy.
+- **One union-find pass.** Path-compressed union-by-size, the same primitive `dedup.py:248` uses. O(α(N)·E) where α is the inverse Ackermann. At N=200 candidates and E ≤ 1000 inter-candidate edges, this is microseconds.
+- **One score-ranked tail iteration.** Same as the existing pack loop, no algorithmic change.
</code_context>
<issue_to_address>
**nitpick (typo):** Consider capitalizing "NumPy" to match the library's official name.
You could update "SQLite-backed numpy" here to "SQLite-backed NumPy" for consistency with the official project name.
```suggestion
- **One `store.edges_for_beliefs(candidate_ids)` call.** Indexed lookup; existing pattern from BFS multi-hop. Cost is dominated by the `IN (...)` clause; at typical candidate-pool size (≤200) this is sub-ms in SQLite-backed NumPy.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| Issue acceptance #4: *"Cluster pass runs inside the retrieve() budget; no separate query."* The cost decomposes into: | ||
|
|
||
| - **One `store.edges_for_beliefs(candidate_ids)` call.** Indexed lookup; existing pattern from BFS multi-hop. Cost is dominated by the `IN (...)` clause; at typical candidate-pool size (≤200) this is sub-ms in SQLite-backed numpy. |
There was a problem hiding this comment.
nitpick (typo): Consider capitalizing "NumPy" to match the library's official name.
You could update "SQLite-backed numpy" here to "SQLite-backed NumPy" for consistency with the official project name.
Suggested change
| - **One `store.edges_for_beliefs(candidate_ids)` call.** Indexed lookup; existing pattern from BFS multi-hop. Cost is dominated by the `IN (...)` clause; at typical candidate-pool size (≤200) this is sub-ms in SQLite-backed numpy. | |
| - **One `store.edges_for_beliefs(candidate_ids)` call.** Indexed lookup; existing pattern from BFS multi-hop. Cost is dominated by the `IN (...)` clause; at typical candidate-pool size (≤200) this is sub-ms in SQLite-backed NumPy. |
|
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 |
Collaborator
Author
|
[claim:review:Toug:2026-05-06T20:18:48Z] |
Collaborator
Author
|
[claim:review:kulili:2026-05-06T20:19:00Z] |
Collaborator
Author
|
[release:review:kulili:2026-05-06T20:19:05Z] |
Collaborator
Author
|
[claim:review:Setr:2026-05-06T20:20:28Z] |
Collaborator
Author
|
[release:review:Setr:2026-05-06T20:20:33Z] |
Retrieval-time pack-stage selection bias toward graph-connected clusters. Distinct from heat-kernel scoring (#150 boosts scores; this changes selection at fixed scores) and dedup union-find (#197 audits at offline-pass time, this runs per-retrieve). Cluster definition: graph-connected components of the post-rank candidate pool, edge_weight >= cluster_edge_weight_floor=0.4 (includes CITES at 0.5 valence, excludes RELATES_TO at 0.3). Pack: two-stage greedy. Stage 1 covers cluster_diversity_target=3 distinct clusters via representatives. Stage 2 fills remaining budget by score, preserving single-fact behaviour. fallback_to_score=True on tight budgets. Bench-gate: positive multi-fact recall + cluster_coverage uplift on a new tests/corpus/v2_0/multi_fact/ fixture; non-regression on the v0.1 retrieve_uplift fixture; <5ms p99 latency at N=200 candidates. Substrate: union-find primitive at dedup.py:248; EDGE_VALENCE weights at models.py:55-66; pack loops at retrieval.py:1048-1085, :1197-1232. No schema changes.
robotrocketscience
force-pushed
the
docs/issue-436-clustering-spec
branch
from
May 6, 2026 20:21
ac82311 to
985c367
Compare
Collaborator
Author
|
[release:review:Toug:2026-05-06T20:22:19Z] |
yoshi280
removed
attn:review
6 tasks
4 tasks
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
2 participants
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.
Spec memo for #436 — Intentional clustering. Closes the recovery-inventory line at
docs/ROADMAP.mdrow 164 (Intentional clustering | v2.0.0).What this PR is
Docs-only. New file at
docs/feature-intentional-clustering.md. Converts the bare issue acceptance sketch into a buildable contract: cluster definition, two-stage pack algorithm, latency analysis, and bench-gate.No code, no schema, no flag wiring yet. This PR moves #436 from
needs-spectobench-gated.Cluster definition (issue acceptance #1)
Graph-connected components of the post-rank candidate pool, edge-weight ≥
cluster_edge_weight_floor=0.4(floor includesCITES@0.5, excludesRELATES_TO@0.3 permodels.py:55-66).Topic-coherent and co-occurrence-derived alternatives rejected:
Pack algorithm
Two-stage diversity-aware greedy, replacing the existing tail-trim (
retrieval.py:1048-1085,:1197-1232):Stage 1 — cluster representatives in descending seed score, until
cluster_diversity_target=3distinct clusters covered or budget exhausted.Stage 2 — fill remaining budget by score, skipping already-included beliefs.
fallback_to_score=Truedefault keeps single-fact recall non-degenerate when the candidate pool has fewer clusters than the diversity target.Latency (issue acceptance #4)
Algorithmic profile: one batched
edges_for_beliefs(candidate_ids)+ one path-compressed union-find pass + score-ranked tail iteration. <1 ms expected at N=200, E=500. A microbench undertests/bench_gate/confirms <5 ms p99 (A4).Reconciliation
DuplicateCluster(Deduplication module (dedup) — v2.0 evaluation #197): same union-find primitive, different relation (offline audit vs retrieval-time). Refactor opportunity.rank → cluster → compress → pack.Substrate
All on
mainas ofe646383:dedup.py:248— path-compressed union-find primitivemodels.py:55-66—EDGE_VALENCEinforming the floor defaultstore.py— needsedges_for_beliefs(candidate_ids)(batched edge fetch; impl PR adds if missing)retrieval.py:118-131— flag-resolution conventionretrieval.py:1048-1085, :1197-1232— pack loops to replacetests/corpus/v2_0/,tests/bench_gate/— corpus + harnessNo new dependencies. No schema changes.
Test plan
github/main— clean.G).cluster_edge_weight_floor=0.4default and thecluster_diversity_target=3default.Refs
docs/ROADMAP.mdrow 164DuplicateCluster)Summary by Sourcery
Document the intentional clustering retrieval feature as a bench-gated spec, defining clustering behavior, pack-stage integration, and acceptance criteria without changing code.
Documentation:
Summary by CodeRabbit