Update embedding adaptation, metrics, CI, and GraphWeave metadata - #8
Conversation
… 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.
|
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:
✨ 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
graphweave/adaptation/adapter.py (1)
369-391: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
embedding_prefixisn't persisted bysave()/load().
finetune()/encode()correctly prependself.embedding_prefixto training texts and inputs (lines 324-328, 359-360), butsave()'s manifest only storesmode/base_model_name, andload()only restores those two fields. Formode_="finetune", a loaded adapter defaults toembedding_prefix=None, soencode()on the reloaded adapter silently stops adding the prefix the underlying model was fine-tuned on — producing embeddings inconsistent with the training distribution, with no warning.🐛 Proposed fix
def save(self, path: str) -> None: out = Path(path) out.mkdir(parents=True, exist_ok=True) - manifest = {"mode": self.mode_, "base_model_name": self.base_model_name} + manifest = { + "mode": self.mode_, + "base_model_name": self.base_model_name, + "embedding_prefix": self.embedding_prefix, + } if self.mode_ == "finetune": self.model_.save(str(out / "model")) elif self.mode_ == "linear": self.linear_.save(str(out / "linear")) (out / "manifest.json").write_text(json.dumps(manifest)) `@classmethod` def load(cls, path: str, base_encoder=None) -> "EmbeddingAdapter": out = Path(path) manifest = json.loads((out / "manifest.json").read_text()) - adapter = cls(base_model_name=manifest.get("base_model_name", "all-MiniLM-L6-v2")) + adapter = cls( + base_model_name=manifest.get("base_model_name", "all-MiniLM-L6-v2"), + embedding_prefix=manifest.get("embedding_prefix"), + ) adapter.mode_ = manifest["mode"]🤖 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/adapter.py` around lines 369 - 391, Persist self.embedding_prefix in the manifest written by EmbeddingAdapter.save, then restore it on the adapter created by EmbeddingAdapter.load before returning it. Preserve the existing behavior when older manifests omit the field by retaining the class default.graphweave/adaptation/pipeline.py (1)
124-155: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winValidate
metadatabefore running triplet collection/fine-tuning, not after.The new
use_metadata_view/metadatacheck (lines 148-155) only runs aftercollect_triplets(real LLM calls) andfinetune(real fine-tuning) have already executed. If a caller forgets to passmetadataon a model fit withuse_metadata_view=True, they pay for the full LLM/triplet/fine-tune pipeline only to have it raise at the very end. Move the check up near the existing fitted-state checks so it fails fast.♻️ Proposed fix
if not getattr(model, "_is_fitted", False): raise ValueError("Model not fitted. Call fit() first.") if model.documents_ is None or model.labels_ is None: raise ValueError( "adapt_and_refit requires the fit-time documents and labels. " "These are not persisted by save()/load() — call this on a " "freshly fit() model, not a reloaded one." ) + 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." + ) from graphweave.core.embeddings import EmbeddingEngine ... - 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." - ) - new_model = GraphWeave(n_topics=model.n_topics, config=copy.deepcopy(model.config))🤖 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 124 - 155, Move the use_metadata_view/metadata validation from the end of adapt_and_refit to immediately after the existing fitted-state checks, before adapter.collect_triplets and adapter.finetune execute. Preserve the current ValueError message and condition: raise when model.config.use_metadata_view is true and metadata is missing, while allowing the pipeline to proceed otherwise.
🧹 Nitpick comments (2)
notebooks/coreset_shorttext_test.ipynb (1)
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid pinning user-facing notebooks to the mutable
reworkbranch.These notebooks will change behavior as the branch moves and may break once it is deleted or force-pushed. Use the published package, an immutable commit, or a release tag instead.
notebooks/coreset_shorttext_test.ipynb#L42-L42: replace@reworkwith a stable package version, tag, or commit.notebooks/cumulative_graphweave_benchmark.ipynb#L36-L39: apply the same reproducible installation source.notebooks/graphweave_full_demo.ipynb#L40-L40: apply the same reproducible installation source.🤖 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, Replace the mutable `@rework` installation source with one reproducible published package version, release tag, or immutable commit in notebooks/coreset_shorttext_test.ipynb lines 42-42, notebooks/cumulative_graphweave_benchmark.ipynb lines 36-39, and notebooks/graphweave_full_demo.ipynb lines 40-40, preserving each notebook’s existing installation behavior.graphweave/labeling/llm_labeler.py (1)
104-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnnotate the shared cache with
ClassVar.
_reasoning_unsupported_modelsis intentionally shared across allLLMLabelerinstances (per the comment), but without aClassVarannotation this looks like (and ruff flags as, RUF012) an accidental mutable class default. Since it's genuinely shared global state, make that explicit — and be aware it also means test isolation matters (see companion comment intests/test_llm_labeler_reasoning.py).♻️ Proposed fix
+from typing import ClassVar + # Populated at runtime: OpenRouter model names that rejected # extra_body={"reasoning": {"enabled": False}} once, so later calls to # the same model skip the doomed attempt instead of paying for it again. - _reasoning_unsupported_models: set = set() + _reasoning_unsupported_models: ClassVar[set] = set()🤖 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 - 108, Update the LLMLabeler class attribute _reasoning_unsupported_models to use a ClassVar annotation, preserving its shared set initialization and runtime caching behavior; import ClassVar from typing if needed.Source: Linters/SAST tools
🤖 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 `@graphweave/labeling/llm_labeler.py`:
- Around line 555-571: Narrow the exception handling around the
reasoning-disabled request in the labeling flow to the provider’s bad-request
exception (such as openai.BadRequestError), so only rejected reasoning
parameters add self.model to _reasoning_unsupported_models; allow timeouts, rate
limits, and other failures to propagate without blacklisting the model, while
preserving the retry without extra_body for genuine parameter rejection.
In `@graphweave/utils/metrics.py`:
- Around line 58-72: Optimize the sliding-window counting in the metrics
computation by restricting word-frequency updates to the normalized keywords
that are later queried, rather than counting every token in each window. In the
same flow, precompute the keyword pair combinations once before iterating over
windows and reuse that collection instead of calling combinations(keywords, 2)
per window; preserve the existing frequency semantics and normalized keyword
keys.
- Around line 262-265: Update the cross-validation logic around cross_val_score
to compute the minimum class count, raise a clear error when it is below 2, and
only then set cv to the bounded value using that count. Keep LogisticRegression
and cross_val_score behavior unchanged for valid class distributions.
In `@pyproject.toml`:
- Line 116: The tracked PKG-INFO metadata is stale after the dependency update
in pyproject.toml. Regenerate PKG-INFO from the updated pyproject.toml, ensuring
the metadata URLs no longer reference topic-extraction-poc and the adaptation
extra includes sentence-transformers>=3.0; apply this for the dependency
declaration at pyproject.toml lines 116 and 155-157.
In `@README.md`:
- Around line 563-569: Verify the consensus graph implementation’s dtype,
pruning behavior, and parallelism limits, then make the mirrored documentation
consistent with those confirmed behaviors. Update README.md lines 563-569 and
PKG-INFO lines 622-628 to remove contradictory float32, lower-threshold, and
unconditional memory-safety claims; also reconcile the higher-threshold
recommendation at PKG-INFO line 794 with the earlier guidance. Keep README.md
and PKG-INFO synchronized.
In `@tests/test_adaptation_pipeline.py`:
- Around line 140-152: Update adapt_and_refit to validate the required metadata
immediately after the fitted-model checks, before triplet collection or
fine-tuning begins. Preserve the ValueError for missing metadata and update
test_metadata_view_raises_when_metadata_not_passed to assert that
_TextOracleLabeler is never called.
In `@tests/test_llm_labeler_reasoning.py`:
- Around line 90-98: Isolate tests from the shared
LLMLabeler._reasoning_unsupported_models state so execution order cannot affect
assertions about extra_body. Update the test setup, preferably with an autouse
fixture that clears this class-level set before and after each test, or ensure
each test uses a unique model identifier while preserving the existing retry
assertions in
test_openrouter_retries_without_reasoning_field_if_model_rejects_it.
---
Outside diff comments:
In `@graphweave/adaptation/adapter.py`:
- Around line 369-391: Persist self.embedding_prefix in the manifest written by
EmbeddingAdapter.save, then restore it on the adapter created by
EmbeddingAdapter.load before returning it. Preserve the existing behavior when
older manifests omit the field by retaining the class default.
In `@graphweave/adaptation/pipeline.py`:
- Around line 124-155: Move the use_metadata_view/metadata validation from the
end of adapt_and_refit to immediately after the existing fitted-state checks,
before adapter.collect_triplets and adapter.finetune execute. Preserve the
current ValueError message and condition: raise when
model.config.use_metadata_view is true and metadata is missing, while allowing
the pipeline to proceed otherwise.
---
Nitpick comments:
In `@graphweave/labeling/llm_labeler.py`:
- Around line 104-108: Update the LLMLabeler class attribute
_reasoning_unsupported_models to use a ClassVar annotation, preserving its
shared set initialization and runtime caching behavior; import ClassVar from
typing if needed.
In `@notebooks/coreset_shorttext_test.ipynb`:
- Line 42: Replace the mutable `@rework` installation source with one reproducible
published package version, release tag, or immutable commit in
notebooks/coreset_shorttext_test.ipynb lines 42-42,
notebooks/cumulative_graphweave_benchmark.ipynb lines 36-39, and
notebooks/graphweave_full_demo.ipynb lines 40-40, preserving each notebook’s
existing installation behavior.
🪄 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: 565ba6fb-bf78-4b1c-83c5-e7c95002d294
📒 Files selected for processing (23)
.github/workflows/ci.ymlPKG-INFOREADME.mdbenchmarks/integration_test_20ng.pygraphweave/adaptation/adapter.pygraphweave/adaptation/correction.pygraphweave/adaptation/keyphrase.pygraphweave/adaptation/pipeline.pygraphweave/adaptation/triplets.pygraphweave/core/__init__.pygraphweave/core/model.pygraphweave/cumulative/README.mdgraphweave/labeling/llm_labeler.pygraphweave/utils/metrics.pynotebooks/coreset_shorttext_test.ipynbnotebooks/cumulative_graphweave_benchmark.ipynbnotebooks/graphweave_full_demo.ipynbpyproject.tomlrun_benchmark.pytests/test_adaptation_keyphrase.pytests/test_adaptation_pipeline.pytests/test_llm_labeler_reasoning.pytests/test_report_themes.py
| # that don't support disabling it just ignore the field. A minority of | ||
| # mandatory-reasoning models reject the field outright instead of ignoring | ||
| # it — the first such rejection is remembered in | ||
| # _reasoning_unsupported_models so later calls to that model skip the | ||
| # doomed attempt instead of paying for a retry every time. | ||
| if self.model in self._reasoning_unsupported_models: | ||
| response = self._client.chat.completions.create(**kwargs) | ||
| else: | ||
| kwargs["extra_body"] = {"reasoning": {"enabled": False}} | ||
| try: | ||
| response = self._client.chat.completions.create(**kwargs) | ||
| except Exception: | ||
| self._reasoning_unsupported_models.add(self.model) | ||
| del kwargs["extra_body"] | ||
| response = self._client.chat.completions.create(**kwargs) | ||
| else: | ||
| response = self._client.chat.completions.create(**kwargs) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Blind except Exception permanently mis-blacklists a model on any failure, not just a reasoning-field rejection.
If the first chat.completions.create(**kwargs) call with extra_body fails for any reason (timeout, rate limit, transient network error), self.model is added to _reasoning_unsupported_models forever — not just for this call. A model that actually supports disabling reasoning would permanently lose that optimization for the rest of the process because of one unrelated transient failure. Ruff also flags this bare except Exception: (BLE001).
Consider narrowing to the provider's actual "bad request" exception type (e.g. openai.BadRequestError) so only genuine parameter rejections are learned, or at minimum add a # noqa: BLE001 with a rationale comment matching the convention already used in graphweave/adaptation/correction.py.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 566-566: Do not catch blind exception: Exception
(BLE001)
🤖 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 555 - 571, Narrow the
exception handling around the reasoning-disabled request in the labeling flow to
the provider’s bad-request exception (such as openai.BadRequestError), so only
rejected reasoning parameters add self.model to _reasoning_unsupported_models;
allow timeouts, rate limits, and other failures to propagate without
blacklisting the model, while preserving the retry without extra_body for
genuine parameter rejection.
Source: Linters/SAST tools
| all_windows = [w for doc in documents for w in windows(tokenize(doc), window_size)] | ||
| n_windows = len(all_windows) | ||
|
|
||
| # Count window frequencies | ||
| word_window_freq = Counter() | ||
| for window in all_windows: | ||
| for word in window: | ||
| word_window_freq[word] += 1 | ||
|
|
||
| # Count co-occurrences (within the same window) | ||
| pair_window_freq = Counter() | ||
| for window in all_windows: | ||
| for w1, w2 in combinations(keywords, 2): | ||
| if w1.lower() in tokens and w2.lower() in tokens: | ||
| pair_doc_freq[(w1.lower(), w2.lower())] += 1 | ||
| if w1.lower() in window and w2.lower() in window: | ||
| pair_window_freq[(w1.lower(), w2.lower())] += 1 |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Sliding-window frequency counting does unnecessary work.
word_window_freq tallies every distinct token seen in any window, even though only the ~10-15 keywords are ever looked up via .get(). pair_window_freq also re-derives combinations(keywords, 2) for every single window instead of once. For long/many documents, sliding one-token-step windows can generate a very large number of windows (len(tokens) - window_size + 1 per doc), so this scales far worse than the previous whole-document counting it replaces. Since compute_coherence runs once per topic on the topic's assigned documents, this can meaningfully slow down evaluation on larger corpora.
♻️ Restrict counting to the keyword set and hoist the pair list
+ keyword_set = {kw.lower() for kw in keywords}
+ keyword_pairs = list(combinations(keywords, 2))
+
all_windows = [w for doc in documents for w in windows(tokenize(doc), window_size)]
n_windows = len(all_windows)
- # Count window frequencies
- word_window_freq = Counter()
- for window in all_windows:
- for word in window:
- word_window_freq[word] += 1
-
- # Count co-occurrences (within the same window)
- pair_window_freq = Counter()
- for window in all_windows:
- for w1, w2 in combinations(keywords, 2):
- if w1.lower() in window and w2.lower() in window:
- pair_window_freq[(w1.lower(), w2.lower())] += 1
+ # Only track frequencies for the keywords we actually need
+ word_window_freq = Counter()
+ pair_window_freq = Counter()
+ for window in all_windows:
+ present = window & keyword_set
+ for word in present:
+ word_window_freq[word] += 1
+ for w1, w2 in keyword_pairs:
+ if w1.lower() in present and w2.lower() in present:
+ pair_window_freq[(w1.lower(), w2.lower())] += 1🤖 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 58 - 72, Optimize the
sliding-window counting in the metrics computation by restricting word-frequency
updates to the normalized keywords that are later queried, rather than counting
every token in each window. In the same flow, precompute the keyword pair
combinations once before iterating over windows and reuse that collection
instead of calling combinations(keywords, 2) per window; preserve the existing
frequency semantics and normalized keyword keys.
| # 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") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a metrics.py . || true
echo "== relevant file excerpt =="
if [ -f graphweave/utils/metrics.py ]; then
sed -n '230,285p' graphweave/utils/metrics.py | cat -n -v
fi
echo "== imports and dependency declarations for sklearn =="
rg -n "sklearn|scikit-learn|LogisticRegression|cross_val_score|compute_downstream_score" graphweave pyproject.toml requirements.txt setup.py setup.cfg poetry.lock Pipfile 2>/dev/null || true
echo "== deterministic behavior probe using installed scikit-learn =="
python3 - <<'PY'
import sys
try:
from sklearn.model_selection import cross_val_score, StratifiedKFold
from sklearn.linear_model import LogisticRegression
from collections import Counter
import numpy as np
except Exception as e:
print("sklearn_import_error:", repr(e))
raise SystemExit(0)
for n_splits in [1, 2, 3, 5]:
try:
cv = StratifiedKFold(n_splits=n_splits, shuffle=True, random_state=42)
model = LogisticRegression(max_iter=1000, random_state=42)
scores = cross_val_score(model, np.array([0,1,2,3,4]).reshape(-1,1), np.array([0,0,1,1,1]), cv=cv, scoring="f1_macro")
print("n_splits", n_splits, "ok", scores)
except Exception as e:
print("n_splits", n_splits, "error", type(e).__name__, str(e))
try:
cv = StratifiedKFold(n_splits=1, shuffle=True, random_state=42)
print("StratifiedKFold n_splits=1 ok")
except Exception as e:
print("StratifiedKFold n_splits=1 error", type(e).__name__, str(e))
PYRepository: nevil-mathew/GraphWeave
Length of output: 7822
🌐 Web query:
scikit-learn cross_val_score StratifiedKFold n_splits must be at least 2 documentation
💡 Result:
In scikit-learn, the n_splits parameter for the StratifiedKFold cross-validator is strictly required to be at least 2 [1][2][3]. This requirement exists because cross-validation by definition involves splitting data into multiple parts to evaluate model performance, and a minimum of 2 splits is necessary to perform a train-test cycle [1][4]. When using StratifiedKFold (or KFold) via cross_val_score, passing an integer to the cv parameter effectively instantiates the splitter with that number of folds [5]. If you provide a value less than 2, the library will raise a ValueError indicating that n_splits must be at least 2 [1][3]. As of scikit-learn version 0.22, the default value for n_splits was updated from 3 to 5 [1][6][3]. You can explicitly configure the number of folds by passing an instance of StratifiedKFold to the cv argument of cross_val_score [5]: from sklearn.model_selection import cross_val_score, StratifiedKFold # Example with 2 folds cv_strategy = StratifiedKFold(n_splits=2) scores = cross_val_score(model, X, y, cv=cv_strategy)
Citations:
- 1: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.StratifiedKFold.html
- 2: https://sklearn.org/1.6/modules/generated/sklearn.model_selection.StratifiedKFold.html
- 3: https://github.com/scikit-learn/scikit-learn/blob/8bf604961/sklearn/model_selection/_split.py
- 4: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.KFold.html?highlight=kfold
- 5: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.cross_val_score
- 6: https://scikit-learn.org/0.22/modules/generated/sklearn.model_selection.StratifiedKFold.html
Reject classification CV when any class has only one sample.
cv = min(5, min(Counter(y_true).values())) still allows cv=1 for a rare singleton class, and sklearn requires n_splits >= 2 for cross_val_score/stratified CV. Raise a clear error before creating the CV splitter when min_class_count < 2.
🤖 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 262 - 265, Update the
cross-validation logic around cross_val_score to compute the minimum class
count, raise a clear error when it is below 2, and only then set cv to the
bounded value using that count. Keep LogisticRegression and cross_val_score
behavior unchanged for valid class distributions.
| "datasets>=2.14.0", | ||
| "accelerate>=0.26.0", | ||
| "torch>=2.0.0", | ||
| "sentence-transformers>=3.0", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "## pyproject outline/size"
wc -l pyproject.toml 2>/dev/null || true
printf '\n## relevant pyproject lines 90-170\n'
sed -n '90,170p' pyproject.toml 2>/dev/null | nl -ba -v90
printf '\n## pkg-info exists and header/extra lines\n'
if [ -f PKG-INFO ]; then
wc -l PKG-INFO
printf '\n--- top ---\n'
sed -n '1,80p' PKG-INFO | nl -ba -v1
if rg -n "Provides-Extra|Requires-Dist|Homepage|Repository|Project-URL|Issues|URL" PKG-INFO > /tmp/pkg_info_fields.txt; then
cat /tmp/pkg_info_fields.txt
else
echo "fields grep found nothing"
fi
else
echo "PKG-INFO missing"
fi
printf '\n## all References/URL strings in pyproject PKG-INFO README\n'
rg -n "nevil-mathew/GraphWeave|GraphWeave|Requires-Dist:\s*sentence-transformers|Provides-Extra:\s*adaptation|adaptation|Project-URL|Homepage|Repository|Issues" pyproject.toml PKG-INFO README.md 2>/dev/null || true
printf '\n## parse simple consistency checks\n'
python3 - <<'PY'
from pathlib import Path
import re
py = Path('pyproject.toml').read_text()
pkg = Path('PKG-INFO').read_text()
for marker, pat in [
('sentence-transformers in pyproject', r'sentence-transformers[\s\S]{0,120}'),
('adaptation extra in pyproject', r'adaptation'),
]:
print(f'{marker}:', 'FOUND' if re.search(pat, py) else 'MISSING')
print('sentence-transformers in PKG-INFO:', 'FOUND' if re.search(r'Requires-Dist:\s+sentence-transformers', pkg) else 'MISSING')
print('adaptation extra in PKG-INFO:', 'FOUND' if re.search(r'Provides-Extra:\s+adaptation', pkg) else 'MISSING')
for label in ['Homepage','Repository','Issues']:
print(f'{label} in PKG-INFO:', 'FOUND' if re.search(r'^'+label+r':\s+', pkg, re.M) else 'MISSING')
PYRepository: nevil-mathew/GraphWeave
Length of output: 282
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "## relevant pyproject lines 90-170"
awk '{printf "%4d\t%s\n", NR, $0}' pyproject.toml | awk 'NR>=1 && NR<=81 {next} {print}' | awk 'NR>=10 && NR<=81'
printf '\n## fields from PKG-INFO if present\n'
if [ -f PKG-INFO ]; then
wc -l PKG-INFO
printf '\n--- top 80 lines ---\n'
awk '{printf "%4d\t%s\n", NR, $0}' PKG-INFO | sed -n '1,80p'
printf '\n--- metadata/project URL and dependency fields ---\n'
grep -nE '^(Provides-Extra|Requires-Dist|Homepage|Repository|Project-URL|Issues|URL):|sentence-transformers|adaptation' PKG-INFO || true
else
echo "PKG-INFO missing"
fi
printf '\n## consistency checks\n'
python3 - <<'PY'
from pathlib import Path
import re
py = Path('pyproject.toml').read_text()
pkg_path = Path('PKG-INFO')
pkg = pkg_path.read_text() if pkg_path.is_file() else ''
checks = [
("sentence-transformers in pyproject", bool(re.search(r'sentence-transformers[\S\s]{0,160}', py))),
("adaptation extra in pyproject", 'adaptation' in py),
]
if pkg:
checks += [
("Provides-Extra: adaptation in PKG-INFO", bool(re.search(r'^Provides-Extra:\s+adaptation', pkg, re.M))),
("Requires-Dist: sentence-transformers in PKG-INFO", bool(re.search(r'^Requires-Dist:\s+sentence-transformers', pkg, re.M))),
]
for label in ['Homepage','Repository','Issues']:
checks.append((f"{label}: in PKG-INFO", bool(re.search(r'^'+label+r':\s+', pkg, re.M))))
checks.append((f"{label}: {label}: GraphWeave in PKG-INFO", bool(re.search(r'^'+label+r':\s+.*GraphWeave', pkg, re.M))))
for name, ok in checks:
print(f"{name}: {'FOUND' if ok else 'MISSING/MISMATCH'}")
PYRepository: nevil-mathew/GraphWeave
Length of output: 14619
Regenerate PKG-INFO after updating pyproject.toml.
The tracked metadata still has stale URLs pointing to topic-extraction-poc, and the adaptation extra does not include sentence-transformers>=3.0 despite pyproject.toml requiring it.
📍 Affects 1 file
pyproject.toml#L116-L116(this comment)pyproject.toml#L155-L157
🤖 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 `@pyproject.toml` at line 116, The tracked PKG-INFO metadata is stale after the
dependency update in pyproject.toml. Regenerate PKG-INFO from the updated
pyproject.toml, ensuring the metadata URLs no longer reference
topic-extraction-poc and the adaptation extra includes
sentence-transformers>=3.0; apply this for the dependency declaration at
pyproject.toml lines 116 and 155-157.
| > **Implementation note:** The co-occurrence matrix is accumulated in int16 (counts never exceed `n_runs` ≤ 32k, so 2 bytes suffice vs float32's 4 bytes) and pruned after each Leiden run — entries that can no longer reach the τ threshold are dropped immediately, so the matrix stays sparse throughout rather than growing to its maximum at the final run. Parallel Leiden runs are capped at 4 concurrent threads regardless of `n_jobs`, preventing 10× peak C-level allocations from all runs landing in memory simultaneously. | ||
|
|
||
| ### When to touch the knobs | ||
|
|
||
| | Situation | What to do | | ||
| |---|---| | ||
| | Any size, default install | **Nothing.** The default is already memory-safe with automatic float32, early pruning, and capped parallelism. | | ||
| | Any size, default install | **Nothing.** The default is already memory-safe with automatic int16, early pruning, and capped parallelism. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve contradictory consensus-memory documentation.
The updated text says the graph path uses int16 and that higher consensus_threshold_tau improves pruning, while the same documentation still describes float32 and lower thresholds as more aggressive. Confirm the implementation, then update all mirrored documentation consistently.
README.md#L563-L569: reconcile the new dtype and “any size” memory-safety claims with the remaining README guidance.PKG-INFO#L622-L628: synchronize the generated description with the final implementation.PKG-INFO#L794-L794: reconcile this higher-threshold recommendation with the earlier lower-threshold guidance.
📍 Affects 2 files
README.md#L563-L569(this comment)PKG-INFO#L622-L628PKG-INFO#L794-L794
🤖 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 `@README.md` around lines 563 - 569, Verify the consensus graph
implementation’s dtype, pruning behavior, and parallelism limits, then make the
mirrored documentation consistent with those confirmed behaviors. Update
README.md lines 563-569 and PKG-INFO lines 622-628 to remove contradictory
float32, lower-threshold, and unconditional memory-safety claims; also reconcile
the higher-threshold recommendation at PKG-INFO line 794 with the earlier
guidance. Keep README.md and PKG-INFO synchronized.
| def test_metadata_view_raises_when_metadata_not_passed(self): | ||
| docs, _labels, embs = _make_corpus() | ||
| cfg = GraphWeaveConfig( | ||
| use_dim_reduction=False, use_iterative_refinement=False, | ||
| n_consensus_runs=3, min_cluster_size=5, n_neighbors=10, | ||
| random_state=42, verbose=False, use_metadata_view=True, | ||
| ) | ||
| model = GraphWeave(config=cfg).fit(docs, embeddings=embs) | ||
| metadata = pd.DataFrame({"category": [f"cat_{i % 3}" for i in range(len(docs))]}) | ||
| model = GraphWeave(config=cfg).fit(docs, embeddings=embs, metadata=metadata) | ||
|
|
||
| with pytest.warns(UserWarning, match="metadata"): | ||
| with pytest.raises(ValueError, match="metadata"): | ||
| adapt_and_refit(model, _TextOracleLabeler(), config=_ADAPT_CONFIG, evaluate=False) | ||
|
|
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Reject missing metadata before adaptation work begins.
The test confirms the eventual ValueError, but adapt_and_refit currently performs triplet collection and fine-tuning before reaching the metadata guard. An invalid call can therefore trigger unnecessary LLM work before failing. Move the validation immediately after the fitted-model checks, and assert that the labeler is not called.
🤖 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_adaptation_pipeline.py` around lines 140 - 152, Update
adapt_and_refit to validate the required metadata immediately after the
fitted-model checks, before triplet collection or fine-tuning begins. Preserve
the ValueError for missing metadata and update
test_metadata_view_raises_when_metadata_not_passed to assert that
_TextOracleLabeler is never called.
| def test_openrouter_retries_without_reasoning_field_if_model_rejects_it(self): | ||
| labeler, fake_client = _labeler_with_fake_client("openrouter") | ||
| fake_client.chat.completions = _RejectsReasoningFieldCompletions() | ||
| result = labeler.call_raw("system", "user") | ||
| assert result == '{"answers": ["B"]}' | ||
| calls = fake_client.chat.completions.calls | ||
| assert len(calls) == 2 | ||
| assert "extra_body" in calls[0] | ||
| assert "extra_body" not in calls[1] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Test relies on LLMLabeler._reasoning_unsupported_models staying empty via file-definition order, not isolation.
This test mutates the shared class-level _reasoning_unsupported_models set (see graphweave/labeling/llm_labeler.py lines 104-108) using the same model="fake/model" string as every other test in this class. It currently passes only because it runs last in file order; under randomized/parallel test execution it would poison state for the earlier assertions expecting extra_body to be present. Reset the set (e.g. via an autouse fixture clearing LLMLabeler._reasoning_unsupported_models) before/after each test, or use a unique model name per test.
🤖 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_llm_labeler_reasoning.py` around lines 90 - 98, Isolate tests from
the shared LLMLabeler._reasoning_unsupported_models state so execution order
cannot affect assertions about extra_body. Update the test setup, preferably
with an autouse fixture that clears this class-level set before and after each
test, or ensure each test uses a unique model identifier while preserving the
existing retry assertions in
test_openrouter_retries_without_reasoning_field_if_model_rejects_it.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation