Add step_timer profiler and refactor consensus algorithm - #1
Add step_timer profiler and refactor consensus algorithm#1nevil-mathew wants to merge 52 commits into
Conversation
…e into model fitting process
… and improve memory stats output
…nd float32 accumulation
…ing with early pruning and sparse representation
…ng clustering runs
…gs support for efficiency
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughTriTopic 2.3.0 adds a step_timer timing utility (with psutil support), instruments core pipeline and graph construction with timed blocks, propagates verbose flags to components, refactors graph-based consensus to sparse upper-triangle accumulation with early pruning and limited parallelism, extends inference APIs to accept precomputed embeddings, and updates README guidance. ChangesTiming and memory profiling infrastructure
Verbose parameter support across components
Consensus Leiden algorithm optimization
Pipeline execution timing instrumentation
Public inference API extensions
Documentation updates
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
coderabbitai
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 399: The guidance is inverted: update the README entries that mention
consensus_threshold_tau and n_consensus_runs so they recommend raising
consensus_threshold_tau (stricter filtering, higher τ) and/or lowering
n_consensus_runs (e.g., 5) to reduce memory/OOM risk rather than lowering τ;
locate and edit the table row text that currently reads "Lower
`n_consensus_runs` ... or lower `consensus_threshold_tau` ..." and change it to
suggest raising `consensus_threshold_tau` (e.g., 0.3 → use a higher value) and
lowering `n_consensus_runs`, and apply the same wording change to the other
occurrence referenced in the comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f0e38867-effa-40fd-b4b1-bd2111ce867e
📒 Files selected for processing (8)
README.mdpyproject.tomltritopic/core/clustering.pytritopic/core/embeddings.pytritopic/core/graph_builder.pytritopic/core/model.pytritopic/utils/__init__.pytritopic/utils/timing.py
| co_occur = None | ||
| for partition in partitions: | ||
| unique_ids = np.unique(partition) | ||
| cluster_map = {cid: idx for idx, cid in enumerate(unique_ids)} | ||
| cols = np.array([cluster_map[c] for c in partition]) | ||
| rows = np.arange(n_nodes) | ||
| data = np.ones(n_nodes, dtype=ones_dtype) | ||
| M = sp_csr((data, (rows, cols)), shape=(n_nodes, len(unique_ids))) | ||
| # M @ M.T is the co-membership matrix for this partition (sparse) | ||
| co_run = M.dot(M.T) | ||
| if co_occur is None: | ||
| co_occur = co_run | ||
| else: | ||
| co_occur = co_occur + co_run | ||
| for r_idx, partition in enumerate(partitions): | ||
| # Group node indices by cluster for this run. | ||
| cluster_to_nodes: dict[int, list[int]] = {} | ||
| for node_idx, cluster_id in enumerate(partition): | ||
| cluster_to_nodes.setdefault(int(cluster_id), []).append(node_idx) | ||
|
|
||
| # Enumerate upper-triangle pairs (i < j, no diagonal) within each cluster. | ||
| # Upper triangle only: the downstream consumer at line 307 reads only | ||
| # coo.row < coo.col, so lower-triangle and diagonal entries are always | ||
| # discarded — no point computing or storing them. | ||
| # This replaces the M @ M.T approach: same counts, no M matrix, no | ||
| # intermediate co_run dense step, half the pairs to store. | ||
| run_rows: list[np.ndarray] = [] | ||
| run_cols: list[np.ndarray] = [] | ||
| for members in cluster_to_nodes.values(): | ||
| if len(members) < 2: | ||
| continue | ||
| m = np.sort(np.asarray(members, dtype=np.int32)) | ||
| ii, jj = np.triu_indices(len(m), k=1) # k=1 skips diagonal | ||
| run_rows.append(m[ii]) | ||
| run_cols.append(m[jj]) | ||
|
|
||
| if not run_rows: | ||
| continue | ||
|
|
||
| r = np.concatenate(run_rows) | ||
| c = np.concatenate(run_cols) | ||
| del run_rows, run_cols | ||
| # int16: counts ∈ [0, n_runs ≤ 32k], 2 bytes vs float32's 4 bytes. | ||
| # Each node is in exactly one cluster per run, so no duplicate (r,c) | ||
| # pairs exist within a single run — .tocsr() handles conversion cleanly. | ||
| co_run = sp_coo( | ||
| (np.ones(len(r), dtype=np.int16), (r, c)), | ||
| shape=(n_nodes, n_nodes), | ||
| ).tocsr() | ||
| del r, c | ||
|
|
||
| co_occur = co_run if co_occur is None else co_occur + co_run | ||
|
|
||
| # Early pruning: after run r_idx, the max a pair can still reach is | ||
| # current_count + runs_remaining. Drop pairs whose ceiling falls | ||
| # below threshold_count — they can never survive the final cut. | ||
| runs_remaining = n_runs - r_idx - 1 | ||
| min_reachable = threshold_count - runs_remaining | ||
| if min_reachable > 1: | ||
| co_occur.data[co_occur.data < min_reachable] = 0 | ||
| co_occur.eliminate_zeros() | ||
|
|
||
| if self.consensus_method == "graph": | ||
| coo = co_occur.tocoo() | ||
| rows, cols = coo.row, coo.col | ||
| freq = coo.data.astype(np.float64) / float(n_runs) | ||
| del coo, co_occur | ||
| return self._consensus_via_leiden_on_graph( | ||
| co_occur, n_nodes, n_runs, partitions | ||
| rows, cols, freq, n_nodes, n_runs, partitions | ||
| ) |
There was a problem hiding this comment.
Handle edge case where co_occur remains None.
If every Leiden partition produces only singleton clusters, co_occur stays None throughout the loop. Line 211 (co_occur.tocoo()) and the hierarchical path (lines 224, 228) will crash with AttributeError.
This is pathological but possible with very high resolution or disconnected graphs. Add a guard before accessing co_occur.
🛡️ Proposed fix
+ # Guard against degenerate case where no pairs were ever co-clustered.
+ if co_occur is None:
+ # Fallback: pick the partition with highest mean ARI.
+ best_score = -1.0
+ best = partitions[0]
+ for p in partitions:
+ avg = float(np.mean([adjusted_rand_score(p, q) for q in partitions]))
+ if avg > best_score:
+ best_score = avg
+ best = p
+ return best
+
if self.consensus_method == "graph":
coo = co_occur.tocoo()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| co_occur = None | |
| for partition in partitions: | |
| unique_ids = np.unique(partition) | |
| cluster_map = {cid: idx for idx, cid in enumerate(unique_ids)} | |
| cols = np.array([cluster_map[c] for c in partition]) | |
| rows = np.arange(n_nodes) | |
| data = np.ones(n_nodes, dtype=ones_dtype) | |
| M = sp_csr((data, (rows, cols)), shape=(n_nodes, len(unique_ids))) | |
| # M @ M.T is the co-membership matrix for this partition (sparse) | |
| co_run = M.dot(M.T) | |
| if co_occur is None: | |
| co_occur = co_run | |
| else: | |
| co_occur = co_occur + co_run | |
| for r_idx, partition in enumerate(partitions): | |
| # Group node indices by cluster for this run. | |
| cluster_to_nodes: dict[int, list[int]] = {} | |
| for node_idx, cluster_id in enumerate(partition): | |
| cluster_to_nodes.setdefault(int(cluster_id), []).append(node_idx) | |
| # Enumerate upper-triangle pairs (i < j, no diagonal) within each cluster. | |
| # Upper triangle only: the downstream consumer at line 307 reads only | |
| # coo.row < coo.col, so lower-triangle and diagonal entries are always | |
| # discarded — no point computing or storing them. | |
| # This replaces the M @ M.T approach: same counts, no M matrix, no | |
| # intermediate co_run dense step, half the pairs to store. | |
| run_rows: list[np.ndarray] = [] | |
| run_cols: list[np.ndarray] = [] | |
| for members in cluster_to_nodes.values(): | |
| if len(members) < 2: | |
| continue | |
| m = np.sort(np.asarray(members, dtype=np.int32)) | |
| ii, jj = np.triu_indices(len(m), k=1) # k=1 skips diagonal | |
| run_rows.append(m[ii]) | |
| run_cols.append(m[jj]) | |
| if not run_rows: | |
| continue | |
| r = np.concatenate(run_rows) | |
| c = np.concatenate(run_cols) | |
| del run_rows, run_cols | |
| # int16: counts ∈ [0, n_runs ≤ 32k], 2 bytes vs float32's 4 bytes. | |
| # Each node is in exactly one cluster per run, so no duplicate (r,c) | |
| # pairs exist within a single run — .tocsr() handles conversion cleanly. | |
| co_run = sp_coo( | |
| (np.ones(len(r), dtype=np.int16), (r, c)), | |
| shape=(n_nodes, n_nodes), | |
| ).tocsr() | |
| del r, c | |
| co_occur = co_run if co_occur is None else co_occur + co_run | |
| # Early pruning: after run r_idx, the max a pair can still reach is | |
| # current_count + runs_remaining. Drop pairs whose ceiling falls | |
| # below threshold_count — they can never survive the final cut. | |
| runs_remaining = n_runs - r_idx - 1 | |
| min_reachable = threshold_count - runs_remaining | |
| if min_reachable > 1: | |
| co_occur.data[co_occur.data < min_reachable] = 0 | |
| co_occur.eliminate_zeros() | |
| if self.consensus_method == "graph": | |
| coo = co_occur.tocoo() | |
| rows, cols = coo.row, coo.col | |
| freq = coo.data.astype(np.float64) / float(n_runs) | |
| del coo, co_occur | |
| return self._consensus_via_leiden_on_graph( | |
| co_occur, n_nodes, n_runs, partitions | |
| rows, cols, freq, n_nodes, n_runs, partitions | |
| ) | |
| co_occur = None | |
| for r_idx, partition in enumerate(partitions): | |
| # Group node indices by cluster for this run. | |
| cluster_to_nodes: dict[int, list[int]] = {} | |
| for node_idx, cluster_id in enumerate(partition): | |
| cluster_to_nodes.setdefault(int(cluster_id), []).append(node_idx) | |
| # Enumerate upper-triangle pairs (i < j, no diagonal) within each cluster. | |
| # Upper triangle only: the downstream consumer at line 307 reads only | |
| # coo.row < coo.col, so lower-triangle and diagonal entries are always | |
| # discarded — no point computing or storing them. | |
| # This replaces the M @ M.T approach: same counts, no M matrix, no | |
| # intermediate co_run dense step, half the pairs to store. | |
| run_rows: list[np.ndarray] = [] | |
| run_cols: list[np.ndarray] = [] | |
| for members in cluster_to_nodes.values(): | |
| if len(members) < 2: | |
| continue | |
| m = np.sort(np.asarray(members, dtype=np.int32)) | |
| ii, jj = np.triu_indices(len(m), k=1) # k=1 skips diagonal | |
| run_rows.append(m[ii]) | |
| run_cols.append(m[jj]) | |
| if not run_rows: | |
| continue | |
| r = np.concatenate(run_rows) | |
| c = np.concatenate(run_cols) | |
| del run_rows, run_cols | |
| # int16: counts ∈ [0, n_runs ≤ 32k], 2 bytes vs float32's 4 bytes. | |
| # Each node is in exactly one cluster per run, so no duplicate (r,c) | |
| # pairs exist within a single run — .tocsr() handles conversion cleanly. | |
| co_run = sp_coo( | |
| (np.ones(len(r), dtype=np.int16), (r, c)), | |
| shape=(n_nodes, n_nodes), | |
| ).tocsr() | |
| del r, c | |
| co_occur = co_run if co_occur is None else co_occur + co_run | |
| # Early pruning: after run r_idx, the max a pair can still reach is | |
| # current_count + runs_remaining. Drop pairs whose ceiling falls | |
| # below threshold_count — they can never survive the final cut. | |
| runs_remaining = n_runs - r_idx - 1 | |
| min_reachable = threshold_count - runs_remaining | |
| if min_reachable > 1: | |
| co_occur.data[co_occur.data < min_reachable] = 0 | |
| co_occur.eliminate_zeros() | |
| # Guard against degenerate case where no pairs were ever co-clustered. | |
| if co_occur is None: | |
| # Fallback: pick the partition with highest mean ARI. | |
| best_score = -1.0 | |
| best = partitions[0] | |
| for p in partitions: | |
| avg = float(np.mean([adjusted_rand_score(p, q) for q in partitions])) | |
| if avg > best_score: | |
| best_score = avg | |
| best = p | |
| return best | |
| if self.consensus_method == "graph": | |
| coo = co_occur.tocoo() | |
| rows, cols = coo.row, coo.col | |
| freq = coo.data.astype(np.float64) / float(n_runs) | |
| del coo, co_occur | |
| return self._consensus_via_leiden_on_graph( | |
| rows, cols, freq, n_nodes, n_runs, partitions | |
| ) |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tritopic/core/clustering.py (1)
61-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
consensus_threshold_tauwhen constructing the clusterer.The config contract says this is a fraction in
[0, 1], but out-of-range values currently fail open:tau > 1prunes every edge and forces the fallback partition, whiletau < 0admits every observed pair. Rejecting invalid input here keeps both consensus paths predictable.Proposed fix
if consensus_method not in ("graph", "hierarchical"): raise ValueError( f"consensus_method must be 'graph' or 'hierarchical', got {consensus_method!r}" ) + if not 0.0 <= consensus_threshold_tau <= 1.0: + raise ValueError( + f"consensus_threshold_tau must be in [0, 1], got {consensus_threshold_tau!r}" + ) self.consensus_method = consensus_method self.consensus_threshold_tau = consensus_threshold_tau🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tritopic/core/clustering.py` around lines 61 - 66, The constructor currently assigns consensus_threshold_tau without validation; add a check in the initializer after validating consensus_method to ensure consensus_threshold_tau is a number within [0, 1] (e.g., isinstance check and 0.0 <= consensus_threshold_tau <= 1.0) and raise ValueError with a clear message if it is out of range or not numeric; update the assignment to self.consensus_threshold_tau only after the validation so that consensus paths remain predictable (refer to the consensus_method and consensus_threshold_tau parameters and the class initializer where they are set).
♻️ Duplicate comments (1)
tritopic/core/clustering.py (1)
210-223:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard the empty co-occurrence case before dereferencing
co_occur.If every Leiden run yields only singleton clusters, the loop never materializes
co_occur, and Line 217 still callstocoo()onNone. The hierarchical branch has the same failure mode, so the new graph fallback never gets a chance to run.Proposed fix
+ if co_occur is None: + best_fallback_score = -1.0 + best = partitions[0] + for p in partitions: + avg = float(np.mean([adjusted_rand_score(p, q) for q in partitions])) + if avg > best_fallback_score: + best_fallback_score = avg + best = p + return best + try: import ctypes ctypes.cdll.LoadLibrary("libc.so.6").malloc_trim(0) except (OSError, AttributeError): pass🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tritopic/core/clustering.py` around lines 210 - 223, The code dereferences co_occur without checking for an empty/None result — guard the empty co-occurrence case by checking if co_occur is None or has no nonzero entries (e.g., co_occur is None or co_occur.nnz == 0) before calling co_occur.tocoo(); if empty, skip the tocoo()/Leiden path and invoke the graph fallback (same behavior needed for the hierarchical branch), ensuring the function returns or falls back to _consensus_via_leiden_on_graph only when co_occur contains data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tritopic/core/clustering.py`:
- Around line 61-66: The constructor currently assigns consensus_threshold_tau
without validation; add a check in the initializer after validating
consensus_method to ensure consensus_threshold_tau is a number within [0, 1]
(e.g., isinstance check and 0.0 <= consensus_threshold_tau <= 1.0) and raise
ValueError with a clear message if it is out of range or not numeric; update the
assignment to self.consensus_threshold_tau only after the validation so that
consensus paths remain predictable (refer to the consensus_method and
consensus_threshold_tau parameters and the class initializer where they are
set).
---
Duplicate comments:
In `@tritopic/core/clustering.py`:
- Around line 210-223: The code dereferences co_occur without checking for an
empty/None result — guard the empty co-occurrence case by checking if co_occur
is None or has no nonzero entries (e.g., co_occur is None or co_occur.nnz == 0)
before calling co_occur.tocoo(); if empty, skip the tocoo()/Leiden path and
invoke the graph fallback (same behavior needed for the hierarchical branch),
ensuring the function returns or falls back to _consensus_via_leiden_on_graph
only when co_occur contains data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a1d6d46-cbc8-4cad-867b-e9de2239d2f7
📒 Files selected for processing (1)
tritopic/core/clustering.py
- Introduced CumulativeTriTopic class for cumulative, batch-wise clustering on top of TriTopic. - Added configuration options for reclustering strategies, triggers, and memory management. - Implemented batch processing with automatic reclustering based on novelty detection and manual triggers. - Created StreamingCorpus utility for generating realistic streaming datasets with LSA embeddings. - Developed evaluation metrics for comparing cumulative clustering against full-batch baselines, including ARI, NMI, and keyword overlap. - Added pluggable recluster strategies: global_refit, coreset, and batch_merge. - Enhanced metrics utility with functions for computing ARI, NMI, and Jaccard overlap for keyword stability.
…ove drift detection
- Introduced GPU acceleration for compute-intensive steps using PyTorch, FAISS, and RAPIDS cuML. - Updated README.md to include GPU installation instructions and performance benefits. - Added `min_cluster_fraction` parameter to TriTopicConfig for better scaling with corpus size. - Refactored clustering methods to utilize effective min_cluster_size based on document count. - Implemented GPU-accelerated cosine similarity and embedding refinement functions. - Updated cumulative benchmark notebook to use new configuration options. - Enhanced graph builder to select between FAISS and HNSW backends based on sample size.
… relevant dependencies
- Introduced LLM-based topic alignment in `alignment.py` with functions `_build_align_prompt` and `_parse_alignment_response`. - Added `llm_align_topics` function to facilitate LLM-driven topic alignment, allowing for flexible mapping of new topics to existing global topics. - Updated `CumulativeTriTopic` to support LLM alignment methods, including "cosine", "llm", and "both", with appropriate configuration options. - Implemented stratified coreset selection in `strategies.py` to ensure representation of rare topics, enhancing robustness against tail-collapse. - Added metrics for evaluating rare topic recall in `evaluation.py`, providing insights into the preservation of small topics during cumulative modeling. - Enhanced `LLMLabeler` to support OpenRouter as a provider, allowing for broader compatibility with LLM services.
- Implemented `llm_merge_topics` method in `TriTopic` and `CumulativeTriTopic` classes to semantically merge topics using a large language model (LLM). - Introduced a new module `llm_merger.py` for handling LLM-driven topic merging, including prompt building and response parsing. - Enhanced `LLMLabeler` with structured output capabilities for LLM calls, allowing for better integration with Google and OpenAI APIs. - Added JSON schema enforcement for structured responses from the LLM, improving the reliability of the merging process. - Updated documentation and added warnings for potential stale states in the cumulative model after merging.
- Implemented sensitivity weights for lightweight-coreset sampling, favoring outliers. - Added tests for sensitivity sampling and micro-cluster coreset strategies. - Enhanced README with details on new coreset selection methods: sensitivity and microcluster. - Updated coreset configuration options to include `coreset_sampling` and `reserve_novel_docs`.
Weighted coresets (stratified_coreset/sensitivity_weights) previously only reached small-cluster pruning, centroids, and keyword extraction — the Leiden partition objective itself drew cluster boundaries as if every sampled point represented one document, blunting the point of the provably-bounded sampling distributions upstream. ConsensusLeiden now switches to RBERVertexPartition with node_sizes=node_weights whenever node weights are present, scoped only to that branch so every unweighted fit (including global_refit) keeps using RBConfigurationVertexPartition exactly as before, with zero behavior change. An earlier attempt that scaled edge weights by w_i*w_j instead of switching objectives was discarded after benchmarking showed it amplifies kNN-graph noise near sparse regions and measurably hurt real coreset fidelity (ARI vs. full-batch baseline dropped); the native node_sizes mechanism avoids that failure mode and raised ARI from ~0.88-0.95 to ~0.92-0.99 across seeds on a realistic streaming benchmark.
Adds a third, opt-in path for choosing the Leiden resolution parameter alongside modularity-maximization and binary-search-to-target-count. Follows ClusterLLM (Zhang, Wang & Shang, EMNLP 2023): candidate resolutions are scored by how well their partitions agree with LLM judgments on sampled same/different-cluster document triplets. - tritopic/labeling/llm_granularity.py: triplet sampling, prompt building, tiered-fallback response parsing, and candidate scoring - TriTopic.tune_resolution_with_llm(): callable after fit(), reuses the existing graph/refresh helpers, never invoked automatically - Unit + integration tests with a mocked labeler (no real API calls) - README section documenting the method and its cost profile Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LaDUfD19BrzSkqbv1bfqbx
- Thread node_weights through llm_select_resolution/_partition_at_resolution so candidate scoring uses the same RBER+node_sizes objective as the final weighted consensus re-fit (previously always unweighted RBConfiguration) - Guard tune_resolution_with_llm against models missing graph_/documents_/ embeddings_ (e.g. after save()/load(), which never persists graph_) with a clear error instead of a deep leidenalg crash - Validate n_candidates/batch_size are >= 1 in llm_select_resolution - Drop unused NearestNeighbors distance outputs in _sample_triplets - Laplace-smooth _triplet_agreement so a candidate informative on only one or two triplets can't outrank one informative across many at a slightly lower but more reliable agreement rate - Replace np.argmax's implicit first-index tie-break with an explicit middle-candidate fallback for tied/all-zero scores - Persist the calibrated resolution back to config.resolution and _clusterer.resolution so later resolution-dependent defaults (build_hierarchy, divide, _auto_resolve_topic_count) stay consistent Adds 10 new tests covering each fix; all verified empirically before being asserted (e.g. Leiden partition invariance for the tie-break test). Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LaDUfD19BrzSkqbv1bfqbx
Verifies the node_weights forwarding fixed in the previous commit (model.py:1871) actually reaches leidenalg end-to-end as the weighted RBER objective across the full call chain (candidate scoring, per-run consensus, and the co-occurrence re-clustering step), not just as a passed-but-unused kwarg. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LaDUfD19BrzSkqbv1bfqbx
New section 5b (opt-in, off by default via TUNE_RESOLUTION_WITH_LLM) demonstrates tune_resolution_with_llm() as an alternative to manually hand-tuning RESOLUTION. Uses the existing OpenRouter labeler pattern already established in this notebook (OPENROUTER_MODEL defaults to google/gemini-2.5-flash-lite; DeepSeek is equally suitable) rather than Claude, since B-or-C triplet judgments don't need a stronger model. Outputs stripped before commit to match the other notebooks in this repo and avoid committing real dataset content into git history. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LaDUfD19BrzSkqbv1bfqbx
…ation-9c6nrj Add LLM-guided granularity calibration (ClusterLLM-style)
- Added a check for empty content in the LLM labeler, raising a ValueError with a descriptive message if the response is empty. - Updated the LLM merger to issue a warning and return all topics as singletons when the LLM response is empty or consists only of whitespace.
- Introduced a minimum cluster size parameter to suppress small clusters during candidate partitioning, aligning with final fit behavior. - Improved triplet sampling efficiency with a global k-NN query, reducing computational cost. - Added diagnostics to capture detailed metrics during resolution selection, including triplet counts and unparsed responses. - Implemented a two-stage resolution selection process: a coarse sweep followed by a fine grid search around the best candidate. - Enhanced triplet response parsing to handle unparsed responses more robustly, avoiding bias in scoring. - Updated documentation to reflect new features and improvements in the LLM granularity approach.
- Introduced a new feature to allow LLM-guided calibration of the RESOLUTION parameter. - Added markdown explanation for the new feature, detailing its benefits and usage. - Implemented code to utilize the LLM for tuning resolution based on boundary case judgments. - Included diagnostics output to assess the effectiveness of the calibration.
…ation docs New tritopic.adaptation subpackage implementing ClusterLLM-style triplet fine-tuning: sample LLM-judged (anchor, positive, negative) triplets from a fitted model, adapt the embedder to them via a pure-numpy linear transform or a real sentence-transformers fine-tune, and refit TriTopic on the result. Also includes the cheaper Few-Shot-Clustering extras (LLM keyphrase expansion, low-confidence correction) and a compare_embedders evaluation harness for judging whether adaptation actually helped on a given corpus. - tritopic/adaptation/: config, triplet sampling/bank/cache, LinearAdapter + EmbeddingAdapter, evaluation harness, keyphrase/correction extras, adapt_and_refit() pipeline - TriTopic.adapt_embeddings_with_llm(): in-place delegate mirroring tune_resolution_with_llm's ergonomics - benchmarks/adaptation_quality_report.py: oracle-labeler POC report (synthetic + real 20 Newsgroups scenarios) - notebooks/embedding_adaptation_demo.ipynb: executed end-to-end demo/test notebook with inline assertions, real 20NG data - 34 new tests across tests/test_adaptation_*.py - pyproject.toml: new `adaptation` extra (datasets, accelerate) - README: document the new feature; expand tune_resolution_with_llm docs with two-stage/bias-mitigation details and a diagnostics example Co-Authored-By: Claude Fable 5 <[email protected]>
- run_benchmark.py: reproduces the README's TriTopic vs BERTopic/NMF/LDA benchmark table (20ng, BBC News, AG News, Arxiv) with a --quick synthetic smoke-test mode for CI. The file was previously referenced by the README but never committed because a blanket `run*` .gitignore rule silently swallowed it; added a `!run_benchmark.py` exception. - Verify quoted phrases in LLM-generated report-theme narratives against the source documents shown to the LLM (tritopic/utils/quote_verification.py), flagging quotes that can't be traced back to a real document so a fabricated participant quote doesn't silently ship in a qualitative research report. Surfaced via ReportTheme.unverified_quotes and an inline warning in export_report()'s Markdown output. - Add GitHub Actions CI: pytest on push/PR (py3.10/3.12), a fast benchmark harness smoke test, and a manually-triggered full benchmark reproduction job. - Fix README code samples using the nonexistent `n_topics_target` kwarg; the actual TriTopic constructor param is `n_topics`.
…ules
CI (.github/workflows/ci.yml):
- persist-credentials: false on all checkout steps
- add a read-only top-level permissions block
- timeout-minutes on the full-benchmark job
run_benchmark.py:
- embedding cache key now includes sample_seed, so a rerun with a different
subsampling seed can't silently reuse embeddings for a different set of docs
tritopic/adaptation/adapter.py:
- _resolve_mode now runs the same accelerate/datasets/sentence-transformers
version check for explicit adapter_mode="finetune" that the "auto" branch
already ran, raising the same actionable message instead of failing later
with a raw ImportError out of _finetune_sentence_transformer
- EmbeddingAdapter.encode()'s normalize flag is now threaded through to
LinearAdapter.transform() in linear mode instead of being ignored
tritopic/adaptation/correction.py:
- reassign_low_confidence drops any topic that a reassignment empties out
entirely before recomputing centroids/probabilities, instead of averaging
an empty embedding slice (was a real NaN-propagation crash, reproduced and
confirmed fixed with a new regression test)
- clarify batch_size's docstring: it chunks iteration only, doesn't batch
LLM requests (kept the signature as-is; a real multi-doc-per-call rewrite
is a bigger change than this pass warrants)
tritopic/adaptation/keyphrase.py:
- generate_keyphrases' cache loader now skips malformed/partial JSONL lines
instead of aborting the whole read
- keyphrase_expand_embeddings no longer blends in encode("") for documents
with an empty keyphrase list; also merges the doc+keyphrase encode calls
into one, halving request overhead for API-backed encoders
tritopic/adaptation/pipeline.py:
- adapt_and_refit carries the original model's explicit n_topics through to
the refit model instead of silently resetting it to "auto"
- warns when use_metadata_view was enabled, since the fit-time metadata
DataFrame isn't persisted on the model and can't be recovered for the refit
tritopic/core/model.py:
- AdaptationConfig is now imported under TYPE_CHECKING so the annotation on
adapt_embeddings_with_llm resolves for static analysis (was a bare
"name not defined" for mypy/Ruff)
Skipped (not still valid / not worth the risk-to-value tradeoff):
- Renaming/removing correction.py's batch_size outright — no call site or
doc example depends on the "real batching" framing; docstring fix covers
the misleading-cost-expectation concern without an API break.
All 232 tests pass (224 pre-existing + 8 new regression tests), including a
reproduction of the correction.py crash against the pre-fix code to confirm
the new test actually exercises the bug.
HF's datasets/huggingface_hub stopped resolving the legacy script-based "ag_news" repo id (HfUriError: repo id must be namespace/name), breaking the full benchmark CI job. Point at fancyzhx/ag_news, the parquet-based mirror with the same text/label schema.
The manual full-benchmark job (4 datasets x 3 seeds x k-grid x 4 models, including BERTopic) was hitting the 45-minute timeout and getting cancelled before finishing, not failing outright.
…mark-error-juhs58 Update AG News dataset source to fancyzhx/ag_news
…teness-ipnli2 Add LLM-guided embedding adaptation module; polish granularity calibration docs
Mirrors the CSV_PATHS config pattern from challenges_clustering_kaggle.ipynb: set CSV_PATHS (or the CSV_PATHS/CSV_PATH env var) to load your own documents from CSV instead of the built-in 20 Newsgroups demo. An optional LABEL_COL enables the ground-truth oracle self-test in Section 3; without it, that section is skipped and the notebook points to the real-LLM path in Section 4.
…-input-kvq90t Add LLM-guided embedding adaptation module; polish granularity calibration docs
- Added a new notebook `embedding_adaptation_kaggle.ipynb` for real fine-tuning on Kaggle using OpenRouter and `all-MiniLM-L6-v2`. - Updated `README.md` to include links to the new notebook and clarify the adaptation process. - Modified `adapt_and_refit` function to include the trained `EmbeddingAdapter` in the report, allowing for persistence of fine-tuned weights. - Added a test to ensure the `EmbeddingAdapter` can be saved and loaded correctly after adaptation.
- Updated CHEAT_SHEET.md to reflect default memory-safe configuration for large datasets. - Changed README.md to specify file locations in a clearer format. - Adjusted README.md to correct the OOM crash mitigation advice. - Modified __init__.py to reorder exports for better organization. - Enhanced graphweave_full_demo.ipynb by clarifying LLM labeling options and comments. - Improved run_benchmark.py to provide clearer warnings when BERTopic is not installed.
- Updated CHEAT_SHEET.md to clarify memory usage for default consensus_method="graph" and legacy hierarchical path. - Revised README.md to emphasize the memory-safe nature of the default path and provide clearer installation instructions for optional features. - Enhanced VISUAL_GUIDE.md to reflect changes in memory usage timelines and consensus steps for both paths. - Adjusted integration tests to remove unnecessary low_memory=True settings when using the default graph consensus. - Modified model configuration to clarify the effect of low_memory on hierarchical consensus only. - Updated cumulative benchmark notebook to reflect changes in consensus method and memory management.
Introduce the GraphWeave brand and top-level API
… improve error handling in adaptation modules - Updated CI to support Python 3.9 in addition to 3.10 and 3.12. - Clarified installation instructions in README for LLM-guided embedding fine-tuning. - Improved error handling in adapter.py and correction.py to provide more informative warnings. - Enhanced keyphrase.py to include normalization options for embeddings. - Updated triplet sampling logic for better performance and clarity. - Added tests for quote verification and reasoning field handling in LLM labeler.
- Changed co-occurrence matrix accumulation from float32 to int16 to optimize memory usage. - Updated documentation to reflect changes in data types and memory safety. - Modified error handling in `adapt_and_refit` to raise ValueError when metadata is not passed, ensuring metadata view is preserved. - Added tests to verify that metadata is correctly handled during adaptation and refitting processes. - Adjusted installation instructions in notebooks to point to the correct repository branch.
Update embedding adaptation, metrics, CI, and GraphWeave metadata
Add LLM-guided embedding adaptation and report quote verification
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
graphweave/core/graph_builder.py (1)
78-113: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winKeep FAISS from preempting hnswlib in the adaptive kNN path.
_select_knn_backendreturnsfaiss_gpu/faiss_cpubefore the hnswlib import branch, so with FAISS available any corpus at/abovehnsw_small_thresholduses the FAISS path. FAISSIndexFlatIPis exact brute-force inner-product search; it does not preserve the documented/Sub-quadratic HNSW fallback (5k–~50k→hnsw_small,≥50k→hnsw_largewith its HNSW tuning). Gate FAISS to an appropriate size band and fall back through hnswlib above it to keep the adaptive behavior intact.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@graphweave/core/graph_builder.py` around lines 78 - 113, Update _select_knn_backend so FAISS is selected only within its intended size band, rather than for every corpus at or above hnsw_small_threshold. For larger corpora, continue through the hnswlib availability and threshold checks so hnsw_small and hnsw_large retain their documented adaptive behavior, while preserving GPU-over-CPU preference within the allowed FAISS range.
🧹 Nitpick comments (7)
graphweave/cumulative/evaluation.py (1)
128-138: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winKeep the OOM warning, but avoid
km.transformas the fix.The
(emb[:, None, :] - centers[None, :, :]) ** 2broadcast creates an(n_samples, n_clusters, n_features)temporary for the full corpus, which can exhaust RAM.km.transform(emb)only produces an(n_samples, n_clusters)distance matrix, which can still be too large. Use chunked nearest-center scoring instead, for example viapairwise_distances_argmin_min(..., metric='euclidean', return_distance=True)and accumulate scores without building the full(N, k, d)array.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@graphweave/cumulative/evaluation.py` around lines 128 - 138, Update _distortion to avoid both broadcasted (n_samples, n_clusters, n_features) allocations and km.transform; compute nearest-center distances in chunks using pairwise_distances_argmin_min or an equivalent chunked approach, and accumulate weighted or unweighted distortion scores incrementally. Preserve the existing full_cost/work_cost ratio and OOM warning behavior.Source: Linters/SAST tools
graphweave/adaptation/pipeline.py (1)
148-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winValidate
metadatabefore the expensive adaptation work.This
use_metadata_viewprecondition is only checked aftercollect_triplets()(billable LLM calls) andadapter.finetune()(training) have already run at Lines 124–129. A caller who forgetsmetadata=pays the full cost only to hit aValueError. Move the check up right afterconfigis resolved so it fails fast.♻️ Proposed reordering
config = config or AdaptationConfig() + if model.config.use_metadata_view and metadata is None: + raise ValueError( + "adapt_and_refit: the original model used use_metadata_view=True, but " + "the fit-time metadata DataFrame is not persisted on the model, so it " + "can't be reconstructed automatically. Pass the same metadata used for " + "the original fit() call via the metadata= argument to preserve the " + "metadata view on refit." + ) documents = model.documents_Then delete the original block at Lines 148–155.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@graphweave/adaptation/pipeline.py` around lines 148 - 155, Move the use_metadata_view/metadata precondition check in adapt_and_refit immediately after config is resolved, before collect_triplets() or adapter.finetune() execute, and remove the later duplicate check while preserving the existing ValueError message and behavior.graphweave/labeling/llm_labeler.py (1)
104-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_reasoning_unsupported_modelsis process-wide shared state.Defining this as a class attribute means every
LLMLabelerinstance mutates the same set. Two labelers pointing at differentbase_urls but using the same model id would share rejection memory, which could suppress the reasoning-toggle attempt on an endpoint that actually supports it. The docstring frames this as intentional caching, so this is only worth revisiting if per-endpoint isolation matters.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@graphweave/labeling/llm_labeler.py` around lines 104 - 107, The _reasoning_unsupported_models cache is shared across all LLMLabeler instances, allowing one endpoint’s rejection to affect another. Move this state from the class attribute into each LLMLabeler instance’s initialization, and update the relevant methods to use the instance-owned set while preserving the existing model-level caching behavior per endpoint.benchmarks/adaptation_quality_report.py (1)
163-167: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winResolve
target_namesonce instead of re-loading the full corpus per category.The inner
fetch_20newsgroups(subset="all")runs once for every entry incats, so the entire ~18k-doc corpus is loaded and parsed 5 times (plus the outer call) only to map category indices to names.target_namesdoesn't depend on thecategoriesfilter, so hoist it out.♻️ Proposed fix
cats = [0, 1, 2, 3, 4] # 5 categories, kept small for a fast local run + target_names = fetch_20newsgroups(subset="all").target_names data = fetch_20newsgroups( subset="all", - categories=[fetch_20newsgroups(subset="all").target_names[c] for c in cats], + categories=[target_names[c] for c in cats], remove=("headers", "footers", "quotes"), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/adaptation_quality_report.py` around lines 163 - 167, Resolve the 20 Newsgroups target names once before constructing the categories argument, then reuse that value when mapping each entry in cats. Update the data-loading flow around fetch_20newsgroups so the full corpus is not fetched inside the per-category list comprehension, while preserving the existing category selection and outer fetch behavior.notebooks/coreset_shorttext_test.ipynb (1)
477-495: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDiagnostics section reaches into
strategies.pyinternals.
ReclusterContext/make_strategy("coreset")are constructed directly here to mirror the model's internal coreset-building logic. This duplicates several constructor parameters inline and will silently drift if the internal strategy signature changes. Acceptable for a benchmark notebook's diagnostics, but consider exposing a small public helper (e.g.CumulativeGraphWeave.debug_working_set()) if this kind of introspection is needed elsewhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@notebooks/coreset_shorttext_test.ipynb` around lines 477 - 495, Expose a public diagnostics helper on CumulativeGraphWeave, such as debug_working_set(), that rebuilds and returns the final coreset working set using the model’s current configuration and state. Update the notebook diagnostics to call this helper instead of constructing ReclusterContext and make_strategy("coreset") directly, while preserving the existing ratio and reporting behavior.tests/test_integration_20ng.py (1)
30-34: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winResolve
target_namesonce instead of re-fetching per category.
fetch_20newsgroups(subset="all")is invoked once for every element ofcats(10–12 full-bunch reconstructions) purely to map indices → names, then again fordata. Even with disk caching this re-parses the whole corpus repeatedly on each fixture build.♻️ Fetch once, reuse names
def _ng20(n_docs: int, cats: list[int], seed: int = 42) -> tuple[list[str], np.ndarray]: """Load a balanced sample of 20NG and return (documents, labels).""" + all_names = fetch_20newsgroups(subset="all").target_names data = fetch_20newsgroups( subset="all", - categories=[fetch_20newsgroups(subset="all").target_names[c] for c in cats], + categories=[all_names[c] for c in cats], remove=("headers", "footers", "quotes"), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_integration_20ng.py` around lines 30 - 34, Update the 20 Newsgroups fixture setup to fetch the full dataset once, store its target_names, and reuse those names when converting cats to category names before constructing data. Replace the per-category fetches inside the categories comprehension while preserving the existing subset and removal options.graphweave/core/model.py (1)
762-767: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGPU
_refine_embeddingsfallback swallows failures silently.Unlike the FAISS/HNSW fallbacks in
graph_builder.py(which print a diagnostic whenverbose), thisexcept Exception: passgives no signal at all ifgpu_refine_embeddingsfails, making GPU-path issues hard to diagnose even withverbose=True.♻️ Suggested fix
try: from graphweave.utils.gpu import gpu_refine_embeddings return gpu_refine_embeddings(original_embeddings, labels, blend_factor) - except Exception: - pass + except Exception as exc: + if self.config.verbose: + print(f" GPU refine failed ({type(exc).__name__}: {exc}); falling back to CPU.")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@graphweave/core/model.py` around lines 762 - 767, Update the GPU fallback in _refine_embeddings so exceptions from gpu_refine_embeddings emit a diagnostic when verbose is enabled, matching the FAISS/HNSW fallback behavior; retain the existing fallback flow after logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 24: Update the Python version matrix in the CI workflow to include the
declared-supported versions 3.11 and 3.13, while preserving the existing 3.9,
3.10, and 3.12 entries so CI covers the full supported 3.9–3.13 range.
In `@graphweave/core/model.py`:
- Around line 806-814: Update the UMAP backend selection in the
dimensionality-reduction flow to require an explicit configuration or CLI opt-in
before importing and using cuML; otherwise always use the CPU umap backend. Add
the corresponding configuration/CLI documentation describing backend selection
and cuML reproducibility caveats, while preserving the existing random-state
behavior.
- Around line 348-369: Update _effective_min_cluster_size and divide so weighted
thresholds use only the documents represented by the current subgraph, not the
full sample_weights_ total. In divide, derive the subgraph’s corresponding
weights, use their summed mass when calculating the threshold, and pass those
same node weights to sub_clusterer.fit_predict so pruning uses consistent
weighted units; preserve existing full-corpus behavior for other callers.
In `@graphweave/utils/metrics.py`:
- Around line 261-266: Update the classification branch around the cv
calculation and cross_val_score call to handle a rarest-class count below two
explicitly, avoiding cross-validation with cv=1 and returning the appropriate
degenerate-case score. Preserve the existing cross-validated F1 behavior when cv
is at least 2.
In `@notebooks/challenges_clustering_kaggle.ipynb`:
- Line 44: Update the notebook’s pip install cell to use the intended public
GraphWeave repository and branch directly, replacing the unresolved
topic-extraction-poc source while preserving the graphweave extras and quiet
installation options.
In `@notebooks/coreset_shorttext_test.ipynb`:
- Line 42: Update the notebook’s GraphWeave pip install command to replace the
unavailable “rework” Git branch with a stable release, tag, or commit SHA that
exists in the repository. Keep the existing extras, repository URL, quiet
output, and filtering behavior unchanged.
In `@notebooks/cumulative_graphweave_benchmark.ipynb`:
- Line 39: Update all notebook installation commands to use the GraphWeave
repository on `@main` or a released tag instead of transient branches. Change
notebooks/cumulative_graphweave_benchmark.ipynb:39 and
notebooks/graphweave_full_demo.ipynb:40 from GraphWeave.git@rework; replace the
old topic-extraction-poc references at
notebooks/embedding_adaptation_demo.ipynb:77 and
notebooks/embedding_adaptation_kaggle.ipynb:44 with the GraphWeave repository on
`@main` or a released tag.
In `@PKG-INFO`:
- Around line 8-10: Regenerate PKG-INFO from the current pyproject.toml so its
project URLs, CI badge metadata, and adaptation extra—including
sentence-transformers>=3.0—match the source configuration; alternatively, remove
PKG-INFO from version control to prevent future drift.
In `@tests/test_integration_20ng.py`:
- Around line 78-90: Update the docstring and nearby comments for the drift
fixture to match cats_old = [1, 2, 3, 4, 5] and cats_new = [9, 10]: describe
five established comp.* categories followed by two emerging rec.sport.*
categories, and remove the incorrect sci.* and eight/four-category claims.
---
Outside diff comments:
In `@graphweave/core/graph_builder.py`:
- Around line 78-113: Update _select_knn_backend so FAISS is selected only
within its intended size band, rather than for every corpus at or above
hnsw_small_threshold. For larger corpora, continue through the hnswlib
availability and threshold checks so hnsw_small and hnsw_large retain their
documented adaptive behavior, while preserving GPU-over-CPU preference within
the allowed FAISS range.
---
Nitpick comments:
In `@benchmarks/adaptation_quality_report.py`:
- Around line 163-167: Resolve the 20 Newsgroups target names once before
constructing the categories argument, then reuse that value when mapping each
entry in cats. Update the data-loading flow around fetch_20newsgroups so the
full corpus is not fetched inside the per-category list comprehension, while
preserving the existing category selection and outer fetch behavior.
In `@graphweave/adaptation/pipeline.py`:
- Around line 148-155: Move the use_metadata_view/metadata precondition check in
adapt_and_refit immediately after config is resolved, before collect_triplets()
or adapter.finetune() execute, and remove the later duplicate check while
preserving the existing ValueError message and behavior.
In `@graphweave/core/model.py`:
- Around line 762-767: Update the GPU fallback in _refine_embeddings so
exceptions from gpu_refine_embeddings emit a diagnostic when verbose is enabled,
matching the FAISS/HNSW fallback behavior; retain the existing fallback flow
after logging.
In `@graphweave/cumulative/evaluation.py`:
- Around line 128-138: Update _distortion to avoid both broadcasted (n_samples,
n_clusters, n_features) allocations and km.transform; compute nearest-center
distances in chunks using pairwise_distances_argmin_min or an equivalent chunked
approach, and accumulate weighted or unweighted distortion scores incrementally.
Preserve the existing full_cost/work_cost ratio and OOM warning behavior.
In `@graphweave/labeling/llm_labeler.py`:
- Around line 104-107: The _reasoning_unsupported_models cache is shared across
all LLMLabeler instances, allowing one endpoint’s rejection to affect another.
Move this state from the class attribute into each LLMLabeler instance’s
initialization, and update the relevant methods to use the instance-owned set
while preserving the existing model-level caching behavior per endpoint.
In `@notebooks/coreset_shorttext_test.ipynb`:
- Around line 477-495: Expose a public diagnostics helper on
CumulativeGraphWeave, such as debug_working_set(), that rebuilds and returns the
final coreset working set using the model’s current configuration and state.
Update the notebook diagnostics to call this helper instead of constructing
ReclusterContext and make_strategy("coreset") directly, while preserving the
existing ratio and reporting behavior.
In `@tests/test_integration_20ng.py`:
- Around line 30-34: Update the 20 Newsgroups fixture setup to fetch the full
dataset once, store its target_names, and reuse those names when converting cats
to category names before constructing data. Replace the per-category fetches
inside the categories comprehension while preserving the existing subset and
removal options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ee272079-5331-403b-a26b-f7d6cdb2e15a
⛔ Files ignored due to path filters (2)
assets/logo.pngis excluded by!**/*.pnguv.lockis excluded by!**/*.lock
📒 Files selected for processing (83)
.github/workflows/ci.yml.gitignoreLEARNING_GUIDE/CHEAT_SHEET.mdLEARNING_GUIDE/CUMULATIVE_QUALITY_REPORT.mdLEARNING_GUIDE/README.mdLEARNING_GUIDE/VISUAL_GUIDE.mdLICENSENOTICE.mdPKG-INFOREADME.mdbenchmarks/adaptation_quality_report.pybenchmarks/cumulative_quality_report.pybenchmarks/integration_test_20ng.pygraphweave/__init__.pygraphweave/adaptation/__init__.pygraphweave/adaptation/_compat.pygraphweave/adaptation/adapter.pygraphweave/adaptation/config.pygraphweave/adaptation/correction.pygraphweave/adaptation/evaluation.pygraphweave/adaptation/keyphrase.pygraphweave/adaptation/pipeline.pygraphweave/adaptation/triplets.pygraphweave/core/__init__.pygraphweave/core/clustering.pygraphweave/core/embeddings.pygraphweave/core/graph_builder.pygraphweave/core/hierarchy.pygraphweave/core/keywords.pygraphweave/core/model.pygraphweave/cumulative/README.mdgraphweave/cumulative/__init__.pygraphweave/cumulative/alignment.pygraphweave/cumulative/cumulative.pygraphweave/cumulative/datasets.pygraphweave/cumulative/evaluation.pygraphweave/cumulative/strategies.pygraphweave/labeling/__init__.pygraphweave/labeling/llm_granularity.pygraphweave/labeling/llm_labeler.pygraphweave/labeling/llm_merger.pygraphweave/utils/__init__.pygraphweave/utils/gpu.pygraphweave/utils/metrics.pygraphweave/utils/quote_verification.pygraphweave/utils/stopwords.pygraphweave/utils/timing.pygraphweave/visualization/__init__.pygraphweave/visualization/plotter.pynotebooks/challenges_clustering_kaggle.ipynbnotebooks/coreset_shorttext_test.ipynbnotebooks/cumulative_graphweave_benchmark.ipynbnotebooks/embedding_adaptation_demo.ipynbnotebooks/embedding_adaptation_kaggle.ipynbnotebooks/graphweave_full_demo.ipynbpyproject.tomlrun_benchmark.pytests/conftest.pytests/test_adaptation_correction.pytests/test_adaptation_evaluation.pytests/test_adaptation_keyphrase.pytests/test_adaptation_linear.pytests/test_adaptation_pipeline.pytests/test_adaptation_triplets.pytests/test_config.pytests/test_coreset_improvements.pytests/test_cumulative.pytests/test_cumulative_llm_align.pytests/test_cumulative_realistic.pytests/test_hierarchy.pytests/test_integration_20ng.pytests/test_llm_granularity.pytests/test_llm_labeler_reasoning.pytests/test_model_basic.pytests/test_quote_verification.pytests/test_report_themes.pytests/test_save_load.pytests/test_stopwords.pytritopic/__init__.pytritopic/core/__init__.pytritopic/labeling/__init__.pytritopic/utils/__init__.pytritopic/visualization/__init__.py
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| python-version: ["3.9", "3.10", "3.12"] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test matrix omits declared-supported Python 3.11 and 3.13.
pyproject/PKG-INFO classifiers and the README advertise support for 3.9–3.13, but CI only exercises 3.9, 3.10, and 3.12. Compat regressions on 3.11/3.13 would ship undetected.
Suggested fix
- python-version: ["3.9", "3.10", "3.12"]
+ python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| python-version: ["3.9", "3.10", "3.12"] | |
| python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 24, Update the Python version matrix in the
CI workflow to include the declared-supported versions 3.11 and 3.13, while
preserving the existing 3.9, 3.10, and 3.12 entries so CI covers the full
supported 3.9–3.13 range.
|
|
||
| def _effective_min_cluster_size(self, n_docs: int) -> int: | ||
| """Return the min_cluster_size to use for a corpus of n_docs documents. | ||
|
|
||
| If ``min_cluster_fraction`` is set, scales with corpus size so the | ||
| threshold is proportional rather than absolute — critical for cumulative | ||
| models whose fitting corpus grows from one batch to the full accumulator. | ||
| The absolute ``min_cluster_size`` acts as the floor. | ||
|
|
||
| With a weighted coreset (``sample_weights_``) the fraction scales with | ||
| the total *represented* mass rather than the coreset point count, so the | ||
| threshold lands in the same units as the summed cluster mass that | ||
| :meth:`ConsensusLeiden._handle_small_clusters` prunes against. | ||
| """ | ||
| if self.config.min_cluster_fraction is not None: | ||
| weights = getattr(self, "sample_weights_", None) | ||
| basis = float(weights.sum()) if weights is not None else n_docs | ||
| return max( | ||
| self.config.min_cluster_size, | ||
| int(self.config.min_cluster_fraction * basis), | ||
| ) | ||
| return self.config.min_cluster_size |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Weighted min_cluster_fraction basis ignores the n_docs argument, breaking divide() for weighted (coreset) models.
When min_cluster_fraction is set and sample_weights_ exists, basis is always self.sample_weights_.sum() — the entire fitted corpus's weight mass — regardless of the n_docs passed in. This happens to be correct for _fit_single_pass/_fit_iterative/_auto_resolve_topic_count, which always call it with len(self.embeddings_) (the full corpus). But divide() (around line 1348) calls self._effective_min_cluster_size(subgraph.vcount()) where subgraph.vcount() is a topic subset size — the returned threshold still reflects whole-corpus weight mass, not the subgraph's. This is compounded by divide()'s sub_clusterer.fit_predict(...) never passing node_weights=, so even a correctly-scoped weighted threshold would be measured against unweighted document counts inside clustering. Net effect: dividing a topic on a weighted/coreset-fit model can silently apply a wildly wrong min_cluster_size, pruning sub-topics incorrectly.
🐛 Proposed fix — scope the weighted basis to the actual documents in question
- def _effective_min_cluster_size(self, n_docs: int) -> int:
+ def _effective_min_cluster_size(self, n_docs: int, weights: np.ndarray | None = None) -> int:
if self.config.min_cluster_fraction is not None:
- weights = getattr(self, "sample_weights_", None)
+ if weights is None:
+ weights = getattr(self, "sample_weights_", None)
basis = float(weights.sum()) if weights is not None else n_docs
return max(
self.config.min_cluster_size,
int(self.config.min_cluster_fraction * basis),
)
return self.config.min_cluster_sizeAnd in divide():
+ sub_weights = (
+ self.sample_weights_[doc_indices] if self.sample_weights_ is not None else None
+ )
sub_labels = sub_clusterer.fit_predict(
- subgraph, min_cluster_size=max(2, self._effective_min_cluster_size(subgraph.vcount()) // 2),
+ subgraph,
+ min_cluster_size=max(2, self._effective_min_cluster_size(subgraph.vcount(), sub_weights) // 2),
resolution=best_res,
+ node_weights=sub_weights,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _effective_min_cluster_size(self, n_docs: int) -> int: | |
| """Return the min_cluster_size to use for a corpus of n_docs documents. | |
| If ``min_cluster_fraction`` is set, scales with corpus size so the | |
| threshold is proportional rather than absolute — critical for cumulative | |
| models whose fitting corpus grows from one batch to the full accumulator. | |
| The absolute ``min_cluster_size`` acts as the floor. | |
| With a weighted coreset (``sample_weights_``) the fraction scales with | |
| the total *represented* mass rather than the coreset point count, so the | |
| threshold lands in the same units as the summed cluster mass that | |
| :meth:`ConsensusLeiden._handle_small_clusters` prunes against. | |
| """ | |
| if self.config.min_cluster_fraction is not None: | |
| weights = getattr(self, "sample_weights_", None) | |
| basis = float(weights.sum()) if weights is not None else n_docs | |
| return max( | |
| self.config.min_cluster_size, | |
| int(self.config.min_cluster_fraction * basis), | |
| ) | |
| return self.config.min_cluster_size | |
| def _effective_min_cluster_size(self, n_docs: int, weights: np.ndarray | None = None) -> int: | |
| """Return the min_cluster_size to use for a corpus of n_docs documents. | |
| If ``min_cluster_fraction`` is set, scales with corpus size so the | |
| threshold is proportional rather than absolute — critical for cumulative | |
| models whose fitting corpus grows from one batch to the full accumulator. | |
| The absolute ``min_cluster_size`` acts as the floor. | |
| With a weighted coreset (``sample_weights_``) the fraction scales with | |
| the total *represented* mass rather than the coreset point count, so the | |
| threshold lands in the same units as the summed cluster mass that | |
| :meth:`ConsensusLeiden._handle_small_clusters` prunes against. | |
| """ | |
| if self.config.min_cluster_fraction is not None: | |
| if weights is None: | |
| weights = getattr(self, "sample_weights_", None) | |
| basis = float(weights.sum()) if weights is not None else n_docs | |
| return max( | |
| self.config.min_cluster_size, | |
| int(self.config.min_cluster_fraction * basis), | |
| ) | |
| return self.config.min_cluster_size |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@graphweave/core/model.py` around lines 348 - 369, Update
_effective_min_cluster_size and divide so weighted thresholds use only the
documents represented by the current subgraph, not the full sample_weights_
total. In divide, derive the subgraph’s corresponding weights, use their summed
mass when calculating the threshold, and pass those same node weights to
sub_clusterer.fit_predict so pruning uses consistent weighted units; preserve
existing full-corpus behavior for other callers.
| f"({self.config.dim_reduction_method.upper()})...") | ||
|
|
||
| if self.config.dim_reduction_method == "umap": | ||
| from umap import UMAP | ||
| try: | ||
| from cuml.manifold import UMAP | ||
| if self.config.verbose: | ||
| print(" > Using cuML UMAP (GPU)") | ||
| except ImportError: | ||
| from umap import UMAP |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate model.py =="
fd -a 'model.py$' . | sed 's#^\./##'
echo "== relevant lines =="
if [ -f graphweave/core/model.py ]; then
sed -n '760,860p' graphweave/core/model.py | cat -n
else
f="$(fd 'model.py$' . | head -n 1)"
sed -n '760,860p' "$f" | cat -n
fi
echo "== search dim_reduction config and docs mentions =="
rg -n "dim_reduction_method|dim reduction|UMAP|random_state|reproducible|reproducibility|byte" . -S
echo "== package requirements references =="
fd 'pyproject.toml|requirements.*|setup.py|environment.*|Dockerfile|docker-compose.yml' . -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1"' sh {}Repository: nevil-mathew/GraphWeave
Length of output: 2038
🌐 Web query:
RAPIDS cuML UMAP random_state reproducibility memory optimizer sampling documentation
💡 Result:
In RAPIDS cuML, the random_state parameter in the UMAP estimator is used to control the random number generator for embedding initialization and optimizer sampling [1][2]. Setting this parameter enables reproducible embeddings, though it comes at the cost of slower training and increased memory usage due to reduced parallelism, which is otherwise used to optimize performance [1][2][3]. Key Considerations for Reproducibility: - Determinism: While setting random_state enables reproducible results, exact determinism is highly dependent on the algorithm and scale. Because high-parallelism GPU kernels often involve non-deterministic floating-point addition ordering, perfect bitwise reproducibility may not be guaranteed on all hardware or for all dataset sizes [1][2][4]. - Limitations: Setting build_algo='nn_descent' will break reproducibility because the NN-Descent algorithm produces non-deterministic K-Nearest Neighbor (KNN) graphs [1][2]. For strict reproducibility, you should avoid this algorithm or use precomputed KNN graphs [4]. - Initialization: Historically, some initialization methods (like spectral initialization) did not always honor the random_state. Recent updates have addressed this to ensure consistent UMAP embeddings [5]. Memory Optimization and Sampling: - Batched NN-Descent: To handle large datasets that do not fit in GPU memory, you can use batched NN-Descent [6]. By setting build_kwds={'n_clusters': N} (where N > 1), you enable batched generation of the KNN graph [7][6]. Increasing the number of clusters reduces peak GPU memory usage at the cost of performance, but it allows for processing much larger datasets [7][6]. - Managed Memory: You can further optimize memory usage by enabling managed memory via the RAPIDS Memory Manager (RMM), which allows UMAP to utilize system memory when GPU capacity is exceeded [7]. - Data Handling: For large inputs, ensure your data is efficiently handled. You can load data on the host (e.g., using mmap_mode='r' with NumPy) and pass data_on_host=True to the fit method to reduce initial device memory pressure [7]. Additionally, recent efforts have focused on reducing peak memory usage by releasing temporary arrays as soon as they are no longer needed during the fit process [8][9].
Citations:
- 1: https://docs.rapids.ai/api/cuml/stable/api/generated/cuml.manifold.umap/
- 2: https://docs.rapids.ai/api/cuml/nightly/api/generated/cuml.manifold.umap/
- 3: https://docs.rapids.ai/api/cuml/nightly/cuml-accel/compatibility/
- 4: [BUG] UMAP: incorrect reproduciblity description for random_state python docs rapidsai/cuml#5147
- 5: [BUG] UMAP random_state doesn't provide consistency rapidsai/cuml#5892
- 6: [FEA] UMAP API for building with batched NN Descent rapidsai/cuml#6022
- 7: [meta] UMAP fixes for 25.04 rapidsai/cuml#6378
- 8: Reduce peak memory in
UMAP.fit/UMAP.fit_transformrapidsai/cuml#6323 - 9: Improve memory efficiency when using precomputed KNN graph in UMAP rapidsai/cuml#7143
🌐 Web query:
cuml.manifold UMAP import error if cuml unavailable umap import fallback
💡 Result:
The library cuml does not automatically fall back to the CPU-based umap-learn library if a standard from cuml.manifold import UMAP import fails due to missing dependencies or environment issues [1][2][3]. If the import from cuml.manifold import UMAP fails, it typically raises an ImportError, ModuleNotFoundError, or an OSError (e.g., related to missing CUDA drivers or incompatible versions) [1][2][3]. The cuml package is designed to be a GPU-accelerated implementation and requires a functional CUDA environment [3][4]. However, there is a distinct mechanism called cuml.accel that allows for GPU acceleration of third-party libraries, including the original umap-learn [5]. 1. cuml.accel Mechanism: If you have a workflow using the standard umap.UMAP (from umap-learn), you can enable the cuml.accel environment to intercept those calls and attempt to dispatch them to the GPU-accelerated cuml implementation [5]. 2. Automatic CPU Fallback: Within the cuml.accel framework, if a specific configuration (e.g., certain metrics or parameters like densmap=True) is not supported on the GPU, cuml will automatically fall back to the CPU-based umap-learn implementation [6][5]. This behavior is silent by default but can be monitored by setting CUML_ACCEL_LOG_LEVEL=info to see "falling back to CPU" messages [5]. 3. Troubleshooting Import Errors: If your import fails, it is usually an environment configuration issue rather than a missing fallback feature. Common causes include: - Incompatible CUDA driver or version mismatches (e.g., trying to use CUDA 12 packages with CUDA 11 drivers) [3]. - Missing required system libraries (e.g., libcudart.so) [3]. - Incompatible versions of cudf, pyarrow, or numba [1][7][6]. If you intend to use UMAP in environments where GPU availability is uncertain, the standard practice is to implement a manual try-except block in your code to switch between the two libraries: try: from cuml.manifold import UMAP except ImportError: from umap import UMAP This ensures your code remains functional regardless of whether cuml is installed or configured correctly [1][2].
Citations:
- 1: [BUG] Issue installing cuML in Databricks rapidsai/cuml#5841
- 2: [BUG] Installing cuml in Jupyter notebook rapidsai/cuml#5330
- 3: [BUG] libcudart.so: cannot open shared object file: No such file or directory rapidsai/cuml#5300
- 4: https://docs.rapids.ai/api/cuml/stable/api/generated/cuml.manifold.umap/
- 5: https://docs.rapids.ai/api/cuml/stable/cuml-accel/examples/third-party-apps/
- 6: https://docs.rapids.ai/api/cuml/nightly/cuml-accel/limitations/
- 7: Installing/Importing cuml HDBSCAN and UMAP issue MaartenGr/BERTopic#1917
🌐 Web query:
cuml manufold UMAP random_state reproducibility issues GitHub 5147 5892
💡 Result:
In the cuML library, reproducibility issues related to the UMAP random_state parameter have been documented and addressed through multiple GitHub issues and pull requests, including those mentioned (Issues #5147 and #5892) [1][2]. Key findings regarding UMAP reproducibility in cuML: 1. Documentation and Precision: Historically, cuML's UMAP documentation incorrectly described random_state as providing only "3 decimal point match" for reproducibility [1]. Recent updates, such as PR #7518, have clarified that random_state is now intended to provide exact consistency, resolving the documentation discrepancies identified in Issue #5147 [1][3]. 2. Consistency Challenges: Issue #5892 highlighted cases where UMAP results could not be replicated even with a fixed random_state [2]. Investigations revealed that certain initialization methods—specifically spectral initialization—were not consistently honoring the random seed in earlier versions [2]. This was addressed by ensuring spectral initialization accepts a seed parameter [2]. 3. Known Limitations and Trade-offs: - Performance Cost: Setting a random_state enables reproducible embeddings but incurs performance costs, including slower training and increased memory usage, because high-parallelism optimizations (which are non-deterministic due to the non-associativity of floating-point addition) must be restricted [4]. - Algorithm Selection: To maintain determinism, cuML automatically switches to build_algo='brute_force_knn' when a random_state is provided, as the default nn_descent algorithm is non-deterministic [4][5]. - Scale: While random_state typically provides exact results for smaller datasets, reproducibility at very large scales remains a complex challenge due to floating-point behavior [1]. 4. Ongoing Maintenance: cuML developers frequently address stability and noise issues associated with seeded UMAP runs (e.g., Issues #6696 and #7176) [5][6]. Users experiencing instability or noise when using a fixed random_state are often advised to verify their init method (e.g., trying init='random' to bypass spectral embedding issues) and ensure they are using the latest version of cuML, as fixes for these behaviors are iteratively merged [2][7][6].
Citations:
- 1: [BUG] UMAP: incorrect reproduciblity description for random_state python docs rapidsai/cuml#5147
- 2: [BUG] UMAP random_state doesn't provide consistency rapidsai/cuml#5892
- 3: Update UMAP
random_statedoc rapidsai/cuml#7518 - 4: https://docs.rapids.ai/api/cuml/stable/api/generated/cuml.manifold.umap/
- 5: [BUG] UMAP with random state seed produces noisy results. rapidsai/cuml#7176
- 6: [BUG] UMAP produces unstable representations with spectral initialization and fixed random state rapidsai/cuml#6696
- 7: [BUG] Deterministic UMAP is not deterministic rapidsai/cuml#5099
Avoid silent UMAP backend autodetection for reproducible embeddings.
This block can pick cuml.manifold.UMAP or umap.UMAP depending only on imports, so the same random_state/config can produce different embeddings between GPU/RAPIDS and CPU-only environments, and cuML’s reproducibility trade-offs aren’t surfaced anywhere in the docs or CLI. Document this caveat, or gate the cuML path behind an explicit opt-in only.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@graphweave/core/model.py` around lines 806 - 814, Update the UMAP backend
selection in the dimensionality-reduction flow to require an explicit
configuration or CLI opt-in before importing and using cuML; otherwise always
use the CPU umap backend. Add the corresponding configuration/CLI documentation
describing backend selection and cuML reproducibility caveats, while preserving
the existing random-state behavior.
| if task == "classification": | ||
| # Cross-validated F1 | ||
| # Cross-validated F1 (cv bounded by the rarest class so small classes don't crash it) | ||
| clf = LogisticRegression(max_iter=1000, random_state=42) | ||
| scores = cross_val_score(clf, features, y_true, cv=5, scoring="f1_macro") | ||
| cv = min(5, min(Counter(y_true).values())) | ||
| scores = cross_val_score(clf, features, y_true, cv=cv, scoring="f1_macro") | ||
| return float(np.mean(scores)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
sklearn cross_val_score StratifiedKFold minimum n_splits requirement
💡 Result:
In scikit-learn, the StratifiedKFold class requires the n_splits parameter to be at least 2 [1][2][3]. This constraint applies whether you instantiate StratifiedKFold directly or pass an integer to the cv parameter of cross_val_score (in which case scikit-learn internally uses StratifiedKFold for classifiers with binary or multiclass targets) [1][4][5]. Setting n_splits to a value less than 2 will raise a ValueError [1][3].
Citations:
- 1: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.StratifiedKFold.html
- 2: https://scikit-learn.org/1.5/modules/generated/sklearn.model_selection.StratifiedKFold.html
- 3: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.StratifiedKFold
- 4: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.cross_val_score
- 5: https://scikit-learn.org/stable/modules/cross_validation.html?highlight=cross_validate
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)graphweave/utils/metrics\.py$|metrics\.py$' || true
echo "== relevant source =="
if [ -f graphweave/utils/metrics.py ]; then
nl -ba graphweave/utils/metrics.py | sed -n '230,280p'
fi
echo "== test behavioral constraints from installed sklearn if available =="
python3 - <<'PY'
import platform, sys
print("python", platform.python_version(), sys.version)
try:
import sklearn
print("sklearn", sklearn.__version__)
except Exception as e:
print("sklearn_import_error", repr(e))
raise SystemExit(0)
import numpy as np
from sklearn.linear_model import LogisticRegression
from sklearn.model_selection import cross_val_score
from collections import Counter
y_true = np.array([0, 1, 2, 2])
counts = Counter(y_true).values()
print("min_count", min(counts))
cv = min(5, min(counts))
print("cv", cv)
try:
scores = cross_val_score(LogisticRegression(max_iter=1000, random_state=42), np.arange(len(y_true)).reshape(-1,1), y_true, cv=cv, scoring="f1_macro")
print("score_mean", float(np.mean(scores)))
except Exception as exc:
print(type(exc).__name__, str(exc))
PYRepository: nevil-mathew/GraphWeave
Length of output: 2038
🌐 Web query:
scikit-learn cross_val_score integer cv classifier uses StratifiedKFold n_splits must >= 2 min classes
💡 Result:
In scikit-learn, when the cv parameter in cross_val_score is an integer (or None), the library automatically selects the cross-validation strategy [1][2][3]. Specifically, if the estimator is a classifier (i.e., it derives from ClassifierMixin) and the target y is binary or multiclass, scikit-learn uses StratifiedKFold [4][1][3]. In all other cases, it uses KFold [1][3]. Regarding StratifiedKFold requirements, the n_splits parameter must be at least 2 [5]. Additionally, because StratifiedKFold attempts to preserve the percentage of samples for each class, each class must appear in the dataset at least as many times as there are folds (n_splits). If a class has fewer samples than n_splits, the stratification cannot be performed as intended, which typically results in an error during the cross-validation process.
Citations:
- 1: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.cross_val_score.html
- 2: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.cross_val_score
- 3: https://github.com/scikit-learn/scikit-learn/blob/95d4f0841d57e8b5f6b2a570312e9d832e69debc/sklearn/model_selection/_validation.py
- 4: https://scikit-learn.org/stable/modules/cross_validation.html
- 5: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.StratifiedKFold.html
Guard the degenerate classification cross-validation case.
When the rarest class has only one sample, cv becomes 1, and scikit-learn raises ValueError because cross_val_score uses StratifiedKFold for classifiers, which requires n_splits >= 2. Handle this path explicitly instead of letting classification scoring crash.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@graphweave/utils/metrics.py` around lines 261 - 266, Update the
classification branch around the cv calculation and cross_val_score call to
handle a rarest-class count below two explicitly, avoiding cross-validation with
cv=1 and returning the appropriate degenerate-case score. Preserve the existing
cross-validated F1 behavior when cv is at least 2.
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "%pip install -q \"graphweave[llm,fast-knn] @ git+https://github.com/nevil-mathew/topic-extraction-poc.git@batch-clustering\"\n", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the referenced repo/branch resolves
git ls-remote https://github.com/nevil-mathew/topic-extraction-poc.git batch-clustering
echo "---"
git ls-remote https://github.com/nevil-mathew/GraphWeave.gitRepository: nevil-mathew/GraphWeave
Length of output: 2067
🌐 Web query:
GitHub repo nevil-mathew topic-extraction-poc branch batch-clustering
💡 Result:
There is no publicly available GitHub repository named "topic-extraction-poc" owned by "nevil-mathew" as of July 24, 2026 [1][2]. While Nevil Mathew is an active developer on GitHub with several public repositories (such as "ai-experiments", "awesome-readme", and "mentoring"), "topic-extraction-poc" is not listed among them [1][2]. Additionally, search results for this specific repository and branch combination yield no matches, suggesting the repository may be private, deleted, or misidentified [1][2]. Nevil Mathew is known for his work on projects like the ELEVATE-Project, where he has contributed to backend systems, cache handling, and CI/CD pipelines [3][4][5]. Research into topic extraction and batch clustering on GitHub frequently points to other projects such as "topicgen", "GitModel", or general "Topic-Modeling" repositories, but none are associated with this user [6][7][8].
Citations:
- 1: https://github.com/nevil-mathew
- 2: https://linkedin.com/in/nevilmathew
- 3: https://myteam.exceeds.ai/profile/nevil-mathew
- 4: Cache issues related to entity_type , entity and notifications ELEVATE-Project/mentoring#1607
- 5: User mentor extension update ELEVATE-Project/mentoring#1577
- 6: https://github.com/namgyu-youn/topicgen
- 7: https://github.com/danielpatrickhug/GitModel
- 8: https://github.com/imanerh/Topic-Modeling
🌐 Web query:
GitHub repo nevil-mathew GraphWeave branch knn-to-ann
💡 Result:
There is no authoritative evidence of a branch named "knn-to-ann" within the GitHub repository "nevil-mathew/GraphWeave". The repository "nevil-mathew/GraphWeave" is associated with a project focused on multi-view graph topic modeling [1]. Research into its repository structure and online activity does not surface any branch by that name, nor does the project's documentation or related scholarly work (such as the research on graph generation via random walk trajectories) reference a transition or component specifically titled "knn-to-ann" [2][3][4]. It is possible that the query refers to a private branch, a misremembered name, or confusion with other software projects that utilize similar terminology (e.g., libraries for approximate nearest neighbor search or graph-based learning) [5][6][7].
Citations:
- 1: https://github.com/nevil-mathew/GraphWeave
- 2: https://github.com/rahulnanda1999/GraphWeave
- 3: https://arxiv.gg/abs/2509.17291
- 4: https://faculty.mccombs.utexas.edu/deepayan.chakrabarti/mywww/papers/pkdd25-graphweave.pdf
- 5: https://github.com/ZJULearning/efanna_graph
- 6: https://graph-learn.readthedocs.io/en/stable/en/gl/graph/graph_operator/knn.html
- 7: https://github.com/kalisam/ggnn
Fix the install source for the notebook.
This pip install points to nevil-mathew/topic-extraction-poc@batch-clustering, which does not resolve as a public repository/branch. Update this cell to install from the intended GraphWeave source and branch directly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@notebooks/challenges_clustering_kaggle.ipynb` at line 44, Update the
notebook’s pip install cell to use the intended public GraphWeave repository and
branch directly, replacing the unresolved topic-extraction-poc source while
preserving the graphweave extras and quiet installation options.
| "id": "90235dcf", | ||
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": "# Uncomment on a fresh environment (e.g. Kaggle/Colab).\n# %pip install -q datasets sentence-transformers plotly scikit-learn\n!pip install \"graphweave[full] @ git+https://github.com/nevil-mathew/GraphWeave.git@rework\" --quiet 2>&1 | grep -E \"^(Successfully|ERROR:|Collecting|error)\" | head -20" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the GitHub repository nevil-mathew/GraphWeave have a branch named "rework"?
💡 Result:
There is no evidence that the GitHub repository nevil-mathew/GraphWeave has a branch named rework. Public information for this repository [1] indicates it contains multiple branches, but none of the available search results identify a branch with that name.
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching notebook:"
git ls-files | grep -F 'notebooks/coreset_shorttext_test.ipynb' || true
echo
echo "Relevant install references:"
if [ -f notebooks/coreset_shorttext_test.ipynb ]; then
rg -n "pip install|graphweave|rework|knn-to-ann|psutil|ReclusterContext|make_strategy" notebooks/coreset_shorttext_test.ipynb || true
fi
echo
echo "Repository references to rework/knn-to-ann:"
rg -n "nevil-mathew/GraphWeave|`@rework`|knn-to-ann|rework" . || true
echo
echo "Git remote/status info:"
git remote -v || true
git branch --show-current 2>/dev/null || trueRepository: nevil-mathew/GraphWeave
Length of output: 2038
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked notebook file:"
git ls-files | grep -F 'notebooks/coreset_shorttext_test.ipynb' || true
echo
echo "Relevant install references in notebook:"
if [ -f notebooks/coreset_shorttext_test.ipynb ]; then
rg -n '"source"\s*:\s*".*pip install|graphweave|`@rework`|knn-to-ann|psutil|ReclusterContext|make_strategy' notebooks/coreset_shorttext_test.ipynb || true
fi
echo
echo "Repository references to rework/knn-to-ann/GraphWeave install:"
rg -n "nevil-mathew/GraphWeave|`@rework`|rework|knn-to-ann" . || true
echo
echo "Remotes/branch:"
git remote -v || true
git branch --show-current || true
git diff --stat || trueRepository: nevil-mathew/GraphWeave
Length of output: 2038
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked notebook file:"
git ls-files 'notebooks/coreset_shorttext_test.ipynb' || true
echo
echo "Relevant install references in notebook:"
if git ls-files 'notebooks/coreset_shorttext_test.ipynb' | grep -qx 'notebooks/coreset_shorttext_test.ipynb'; then
rg -n '"source"\s*:\s*".*pip install|graphweave|`@rework`|knn-to-ann|psutil|ReclusterContext|make_strategy' notebooks/coreset_shorttext_test.ipynb || true
fi
echo
echo "Repository references to rework/knn-to-ann/GraphWeave install:"
rg -n "nevil-mathew/GraphWeave|`@rework`|rework|knn-to-ann" . || true
echo
echo "Remotes/branch:"
git remote -v || true
git branch --show-current || true
git diff --stat || trueRepository: nevil-mathew/GraphWeave
Length of output: 2038
Install from a stable commit, not the missing @rework branch.
graphweave[full] @ git+...@rework points to a branch that is not present on GitHub; re-running this notebook fails during install. Pin a release/tag or commit SHA instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@notebooks/coreset_shorttext_test.ipynb` at line 42, Update the notebook’s
GraphWeave pip install command to replace the unavailable “rework” Git branch
with a stable release, tag, or commit SHA that exists in the repository. Keep
the existing extras, repository URL, quiet output, and filtering behavior
unchanged.
| "# This notebook's current configuration doesn't use RAPIDS, so those conflicts\n", | ||
| "# have no effect here.\n", | ||
| "\n", | ||
| "!pip install \"graphweave[full] @ git+https://github.com/nevil-mathew/GraphWeave.git@rework\" --quiet 2>&1 | grep -E \"^(Successfully|ERROR:|Collecting|error)\" | head -20\n", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Install commands pin to transient branches / the old repo name and won't resolve post-merge. All four notebooks pip install GraphWeave from feature branches, and two still reference the pre-rebrand repo topic-extraction-poc. After this PR merges these installs will fetch the wrong (or non-existent) package. Point them at the GraphWeave repo on @main (or a released tag).
notebooks/cumulative_graphweave_benchmark.ipynb#L39: changeGraphWeave.git@reworkto@main/a tag.notebooks/graphweave_full_demo.ipynb#L40: changeGraphWeave.git@reworkto@main/a tag.notebooks/embedding_adaptation_demo.ipynb#L77: replacetopic-extraction-poc.git@batch-clusteringwith the GraphWeave repo on@main/a tag.notebooks/embedding_adaptation_kaggle.ipynb#L44: replacetopic-extraction-poc.git@llm-embedding-adaptationwith the GraphWeave repo on@main/a tag.
📍 Affects 4 files
notebooks/cumulative_graphweave_benchmark.ipynb#L39-L39(this comment)notebooks/graphweave_full_demo.ipynb#L40-L40notebooks/embedding_adaptation_demo.ipynb#L77-L77notebooks/embedding_adaptation_kaggle.ipynb#L44-L44
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@notebooks/cumulative_graphweave_benchmark.ipynb` at line 39, Update all
notebook installation commands to use the GraphWeave repository on `@main` or a
released tag instead of transient branches. Change
notebooks/cumulative_graphweave_benchmark.ipynb:39 and
notebooks/graphweave_full_demo.ipynb:40 from GraphWeave.git@rework; replace the
old topic-extraction-poc references at
notebooks/embedding_adaptation_demo.ipynb:77 and
notebooks/embedding_adaptation_kaggle.ipynb:44 with the GraphWeave repository on
`@main` or a released tag.
| Project-URL: Homepage, https://github.com/nevil-mathew/topic-extraction-poc | ||
| Project-URL: Repository, https://github.com/nevil-mathew/topic-extraction-poc | ||
| Project-URL: Issues, https://github.com/nevil-mathew/topic-extraction-poc/issues |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
PKG-INFO is stale and inconsistent with pyproject.toml.
This generated metadata still points at the old repo (topic-extraction-poc) whereas pyproject.toml/README.md use nevil-mathew/GraphWeave (also affects the CI badge at line 91). The embedded adaptation extra here (Lines 66-69) also lacks the sentence-transformers>=3.0 pin that pyproject.toml line 116 now adds. Since PKG-INFO is produced at build time, a committed copy will keep drifting from the source of truth — regenerate it from the current pyproject.toml, or drop it from version control entirely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PKG-INFO` around lines 8 - 10, Regenerate PKG-INFO from the current
pyproject.toml so its project URLs, CI badge metadata, and adaptation
extra—including sentence-transformers>=3.0—match the source configuration;
alternatively, remove PKG-INFO from version control to prevent future drift.
| """ | ||
| Drift scenario: 8 categories for the first 2 batches, | ||
| then 4 new categories appear in batch 3. | ||
|
|
||
| Established: categories 1-5 (comp.* topics) | ||
| Emerging: categories 11-14 (sci.crypt, sci.electronics, sci.med, sci.space) | ||
| These groups are vocabulary-distinct enough for LSA to detect drift cleanly. | ||
| Returns (batch_docs, batch_embs, batch_labels). | ||
| """ | ||
| # comp.* is vocabulary-technical; rec.sport.* is sports — very different in LSA space, | ||
| # giving ~22 % novelty and a clean drift signal even at 64-dim. | ||
| cats_old = [1, 2, 3, 4, 5] # comp.graphics, comp.os.*, comp.sys.*, comp.windows.* | ||
| cats_new = [9, 10] # rec.sport.baseball, rec.sport.hockey |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Docstring/comments contradict the fixture's actual categories.
The docstring states "8 categories for the first 2 batches, then 4 new categories appear in batch 3" and "Emerging: categories 11-14 (sci.crypt, sci.electronics, sci.med, sci.space)", but the code uses cats_old = [1, 2, 3, 4, 5] and cats_new = [9, 10] (rec.sport.*). Align the prose with the code to avoid misleading maintainers about what drift this exercises.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_integration_20ng.py` around lines 78 - 90, Update the docstring
and nearby comments for the drift fixture to match cats_old = [1, 2, 3, 4, 5]
and cats_new = [9, 10]: describe five established comp.* categories followed by
two emerging rec.sport.* categories, and remove the incorrect sci.* and
eight/four-category claims.
2 participants
Summary by CodeRabbit