Add LLM-guided embedding adaptation module; polish granularity calibration docs - #5
Merged
nevil-mathew merged 1 commit intoJul 5, 2026
Conversation
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.
📝 WalkthroughWalkthroughIntroduces a new tritopic.adaptation package for LLM-guided embedding fine-tuning (triplet sampling, linear/fine-tune adapters, evaluation, keyphrase/correction extras), wires it into TriTopic.adapt_embeddings_with_llm, adds a quote-verification utility integrated into report generation, and adds a CI workflow plus standalone benchmark reproduction script with docs and dependency updates. ChangesLLM-Guided Embedding Adaptation
Quote Verification for Report Narratives
CI Workflow and Benchmark Reproduction
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant TriTopic
participant adapt_and_refit
participant EmbeddingAdapter
participant TripletBank
participant LinearAdapter
TriTopic->>adapt_and_refit: adapt_embeddings_with_llm(labeler, config)
adapt_and_refit->>EmbeddingAdapter: collect_triplets(documents, embeddings, labels)
EmbeddingAdapter->>TripletBank: collect()
TripletBank-->>EmbeddingAdapter: judgments (train/holdout)
adapt_and_refit->>EmbeddingAdapter: finetune(documents, embeddings, bank)
EmbeddingAdapter->>LinearAdapter: fit(embeddings, judgments)
LinearAdapter-->>EmbeddingAdapter: trained W
adapt_and_refit->>EmbeddingAdapter: encode(documents)
EmbeddingAdapter-->>adapt_and_refit: adapted embeddings
adapt_and_refit->>TriTopic: fit(adapted embeddings)
adapt_and_refit-->>TriTopic: new model, report
sequenceDiagram
participant model as TriTopic.model
participant narrative as narrative_generation
participant verify as verify_quotes
participant export as export_report
narrative->>verify: verify_quotes(narrative, doc_texts)
verify-->>narrative: unverified_quotes
narrative->>model: ReportTheme(unverified_quotes=...)
export->>model: scan report_themes_
export-->>export: insert Review needed notice and warning markers
Related Issues: None referenced. Related PRs: None referenced. Suggested labels: enhancement, feature, ci, documentation, tests Suggested reviewers: nevil-mathew 🐰 A rabbit hops through triplets, bank, and adapter's gate, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
nevil-mathew
changed the base branch from
batch-clustering
to
llm-embedding-adaptation
July 5, 2026 04:14
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
tritopic/adaptation/adapter.py (1)
285-296: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport the trainer API from the public package
SentenceTransformerTrainer,SentenceTransformerTrainingArguments, andlossesare re-exported bysentence_transformers; thesentence_transformers.sentence_transformerimport path can be removed.🤖 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/adaptation/adapter.py` around lines 285 - 296, The import fallback in the adapter is using an internal module path for the trainer API, but `SentenceTransformerTrainer`, `SentenceTransformerTrainingArguments`, and `losses` should be imported directly from the public `sentence_transformers` package. Update the import block in `adaptation.adapter` to remove the `sentence_transformers.sentence_transformer` branch and keep only the public-package import path for these symbols.README.md (1)
103-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInstall comment omits
torchdependency.
pyproject.toml's newadaptationextra pulls indatasets,accelerate, andtorch>=2.0.0, but this comment only mentions the first two —torchis a substantial download worth calling out.📝 Proposed fix
-# With LLM-guided embedding fine-tuning (adds datasets, accelerate) +# With LLM-guided embedding fine-tuning (adds datasets, accelerate, torch) pip install tritopic[adaptation]🤖 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 103 - 105, The installation note for the tritopic[adaptation] extra is missing the torch dependency, so update the README comment near the adaptation install command to mention that this extra also installs torch>=2.0.0 alongside datasets and accelerate. Keep the wording aligned with the existing install guidance and make sure the README matches the dependencies declared in pyproject.toml for the adaptation extra.benchmarks/adaptation_quality_report.py (3)
116-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused
new_modelbinding (Ruff RUF059).🧹 Proposed fix
- new_model, report = adapt_and_refit( + _new_model, report = adapt_and_refit( model, labeler, config=_adapt_cfg(), evaluate=True, labels_true=labels_true )🤖 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` at line 116, The `new_model` result from `adapt_and_refit` is not used, triggering Ruff RUF059. Update the assignment in the adaptation quality report flow to ignore that return value explicitly, and keep only the `report` binding at the call site in `adaptation_quality_report.py` so the intent is clear and the unused variable warning is removed.Source: Linters/SAST tools
162-171: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant full-dataset fetch just to read category names.
fetch_20newsgroups(subset="all")is called once (line 165) purely to index.target_names, then again (line 163) with thecategoriesfilter — loading/parsing the full ~18K-document dataset twice. 20 Newsgroups'target_namesare stable; consider fetching once and slicing, or hardcoding the category name list.⚡ Proposed fix
- cats = [0, 1, 2, 3, 4] # 5 categories, kept small for a fast local run - data = fetch_20newsgroups( - subset="all", - categories=[fetch_20newsgroups(subset="all").target_names[c] for c in cats], - remove=("headers", "footers", "quotes"), - ) + cats = [0, 1, 2, 3, 4] # 5 categories, kept small for a fast local run + all_target_names = fetch_20newsgroups(subset="all").target_names + data = fetch_20newsgroups( + subset="all", + categories=[all_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 162 - 171, Avoid calling fetch_20newsgroups(subset="all") twice in the same setup block; the second call is only being used to read .target_names. In adaptation_quality_report.py, update the data-loading logic around the cats list and fetch_20newsgroups call so the category names are obtained without reloading the full dataset a second time, either by fetching once and reusing target_names or by using a fixed category-name list before the filtered fetch.
50-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
OracleLabelerhere duplicates the identical class defined innotebooks/embedding_adaptation_demo.ipynb. Consider extracting a shared test-utility (e.g. undertritopic.adaptationtest helpers) both can import, since the regex depends on the internal triplet-prompt format and any drift would silently break both.🤖 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 50 - 75, The OracleLabeler implementation is duplicated here and in the notebook, so factor the shared logic into a common test utility and import it from both places. Move the triplet-parsing and snippet-to-label matching behavior from OracleLabeler into a reusable helper under the adaptation test helpers, then have this benchmark code use that shared symbol so the regex and prompt-format assumptions stay centralized. Keep the existing OracleLabeler entry point but make it delegate to the shared helper to avoid drift.notebooks/embedding_adaptation_demo.ipynb (1)
108-140: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSilent misconfiguration when
LABEL_COLis set but missing.If a user sets
LABEL_COLto a typo'd or wrong column name,LABEL_COL and LABEL_COL in raw.columnssilently evaluates false andtrue_labelsbecomesNone— Section 3's oracle self-test is then silently skipped with the generic "no ground-truth" message, masking a configuration typo.💡 Proposed fix
- if LABEL_COL and LABEL_COL in raw.columns: + if LABEL_COL and LABEL_COL in raw.columns: codes, categories_index = pd.factorize(raw[LABEL_COL]) true_labels = codes categories = categories_index.tolist() + elif LABEL_COL: + raise ValueError(f"LABEL_COL={LABEL_COL!r} not found in columns: {list(raw.columns)}") else: true_labels = None categories = None🤖 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/embedding_adaptation_demo.ipynb` around lines 108 - 140, The CSV loading block in the notebook silently treats a configured LABEL_COL as “not provided” when the column name is wrong, which hides typos. Update the logic around raw, true_labels, and categories so that if LABEL_COL is set but not present in raw.columns, it raises a clear error instead of falling through to the “no ground-truth” path. Keep the existing behavior only when LABEL_COL is unset, and preserve the current factorize-based label handling when the column exists.pyproject.toml (1)
96-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
adaptationinfull.datasetsandaccelerateare pinned twice here; referencingtritopic[adaptation]would keep the extras in sync.🤖 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` around lines 96 - 115, The full extra duplicates the datasets and accelerate pins already defined in adaptation, so keep them in sync by reusing the adaptation extra from the full extra instead of repeating those dependencies. Update the full extras block in pyproject.toml so the shared Trainer-related requirements are sourced through tritopic[adaptation], while preserving the other full-only packages like anthropic, openai, and torch.tritopic/adaptation/_compat.py (1)
30-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSort
__all__per Ruff RUF022.♻️ Suggested fix
__all__ = [ - "build_triplet_prompt", - "TRIPLET_SCHEMA", - "parse_triplet_response", - "sample_triplets_fast", - "sample_triplets_informed", + "TRIPLET_SCHEMA", + "build_triplet_prompt", + "parse_triplet_response", + "sample_triplets_fast", + "sample_triplets_informed", ]🤖 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/adaptation/_compat.py` around lines 30 - 36, The __all__ export list in _compat.py is not sorted, which triggers Ruff RUF022. Reorder the entries in __all__ alphabetically while keeping the same exported symbols, and leave the list structure intact so the module exports remain unchanged.Source: Linters/SAST tools
tritopic/adaptation/triplets.py (1)
128-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate k-NN same/diff extraction logic.
_nearest_same_diffreimplements the exact same same-cluster/diff-cluster neighbour extraction loop as_sample_triplets_fastintritopic/labeling/llm_granularity.py(re-exported via_compat.sample_triplets_fast) — same normalization, sameNearestNeighborscall, same inner loop. Consider factoring the shared loop out of_sample_triplets_fastinllm_granularity.pyand re-exporting it through_compat.py, consistent with the detachability boundary this package already establishes.🤖 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/adaptation/triplets.py` around lines 128 - 164, The same nearest-neighbor same/different triplet extraction logic is duplicated in `_nearest_same_diff`, matching `_sample_triplets_fast` in `llm_granularity.py` and its `_compat.sample_triplets_fast` export. Refactor the shared normalization/NearestNeighbors/inner selection loop into a single helper in `llm_granularity.py`, then reuse or re-export it through `_compat.py` so `_nearest_same_diff` can call the shared implementation instead of maintaining a parallel copy.tritopic/adaptation/config.py (1)
14-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider validating config ranges in
__post_init__.Fields like
holdout_frac,entropy_top_frac, andn_tripletshave no bounds checking. E.g.holdout_frac >= 1.0would push every judgment into the holdout split (empty train set) orholdout_frac < 0would push everything into train (empty holdout) — both silently degradeadapt_and_refit's evaluation rather than failing fast with a clear error.♻️ Suggested validation
+ def __post_init__(self) -> None: + if not 0.0 <= self.holdout_frac < 1.0: + raise ValueError(f"holdout_frac must be in [0, 1), got {self.holdout_frac}") + if not 0.0 < self.entropy_top_frac <= 1.0: + raise ValueError(f"entropy_top_frac must be in (0, 1], got {self.entropy_top_frac}") + if self.n_triplets <= 0: + raise ValueError(f"n_triplets must be positive, got {self.n_triplets}")🤖 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/adaptation/config.py` around lines 14 - 49, Add validation to AdaptationConfig, ideally in a __post_init__ method, to fail fast on invalid ranges instead of silently producing empty train/holdout splits. Check key fields like holdout_frac, entropy_top_frac, n_triplets, and related numeric settings for sensible bounds, and raise clear errors when values are out of range. Keep the validation close to the AdaptationConfig dataclass so adapt_and_refit and downstream callers always receive a valid configuration.
🤖 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:
- Around line 39-40: The CI test command is over-excluding by ignoring the
entire 20NG integration module, which also drops the CI-safe ng20_small
coverage. Update the workflow’s test step so the pytest call on
test_integration_20ng keeps the module included and only filters out slow cases,
using the existing test command pattern in the CI job rather than --ignore on
the whole file.
In `@tritopic/adaptation/keyphrase.py`:
- Around line 147-152: The concat_encode branch in keyphrase adaptation ignores
the normalize option, unlike the average branch. Update the concat_encode path
in the keyphrase encoding flow to apply the same normalization behavior when
normalize=True, likely by normalizing the result after encoder.encode before
returning it, so both branches in the encode logic behave consistently.
In `@tritopic/adaptation/triplets.py`:
- Around line 265-330: The cache in the triplet adaptation flow reuses judgments
based only on content hash, so cached raw row indices can be replayed against a
reordered or partially different corpus. Update the caching path in the triplet
judgment logic around _load_cache_index, _append_cache, and the
resolved/judgment construction to store a corpus fingerprint or stable document
IDs alongside each TripletJudgment, then verify that fingerprint before
accepting a cache hit and invalidate or skip mismatched entries.
In `@tritopic/core/model.py`:
- Around line 1986-1999: The adaptation path in the model update flow leaves the
fitted object with the original encoder, so post-adaptation inference can embed
new documents in the wrong space. In the method that calls adapt_and_refit() and
then does self.__dict__.update(new_model.__dict__), make sure the adapted
encoder from new_model is persisted onto the returned model as well, especially
the _embedding_engine used by transform() and transform_proba(). Verify the
updated model keeps the adapted embedding engine consistently with the adapted
centroids/distance state.
---
Nitpick comments:
In `@benchmarks/adaptation_quality_report.py`:
- Line 116: The `new_model` result from `adapt_and_refit` is not used,
triggering Ruff RUF059. Update the assignment in the adaptation quality report
flow to ignore that return value explicitly, and keep only the `report` binding
at the call site in `adaptation_quality_report.py` so the intent is clear and
the unused variable warning is removed.
- Around line 162-171: Avoid calling fetch_20newsgroups(subset="all") twice in
the same setup block; the second call is only being used to read .target_names.
In adaptation_quality_report.py, update the data-loading logic around the cats
list and fetch_20newsgroups call so the category names are obtained without
reloading the full dataset a second time, either by fetching once and reusing
target_names or by using a fixed category-name list before the filtered fetch.
- Around line 50-75: The OracleLabeler implementation is duplicated here and in
the notebook, so factor the shared logic into a common test utility and import
it from both places. Move the triplet-parsing and snippet-to-label matching
behavior from OracleLabeler into a reusable helper under the adaptation test
helpers, then have this benchmark code use that shared symbol so the regex and
prompt-format assumptions stay centralized. Keep the existing OracleLabeler
entry point but make it delegate to the shared helper to avoid drift.
In `@notebooks/embedding_adaptation_demo.ipynb`:
- Around line 108-140: The CSV loading block in the notebook silently treats a
configured LABEL_COL as “not provided” when the column name is wrong, which
hides typos. Update the logic around raw, true_labels, and categories so that if
LABEL_COL is set but not present in raw.columns, it raises a clear error instead
of falling through to the “no ground-truth” path. Keep the existing behavior
only when LABEL_COL is unset, and preserve the current factorize-based label
handling when the column exists.
In `@pyproject.toml`:
- Around line 96-115: The full extra duplicates the datasets and accelerate pins
already defined in adaptation, so keep them in sync by reusing the adaptation
extra from the full extra instead of repeating those dependencies. Update the
full extras block in pyproject.toml so the shared Trainer-related requirements
are sourced through tritopic[adaptation], while preserving the other full-only
packages like anthropic, openai, and torch.
In `@README.md`:
- Around line 103-105: The installation note for the tritopic[adaptation] extra
is missing the torch dependency, so update the README comment near the
adaptation install command to mention that this extra also installs torch>=2.0.0
alongside datasets and accelerate. Keep the wording aligned with the existing
install guidance and make sure the README matches the dependencies declared in
pyproject.toml for the adaptation extra.
In `@tritopic/adaptation/_compat.py`:
- Around line 30-36: The __all__ export list in _compat.py is not sorted, which
triggers Ruff RUF022. Reorder the entries in __all__ alphabetically while
keeping the same exported symbols, and leave the list structure intact so the
module exports remain unchanged.
In `@tritopic/adaptation/adapter.py`:
- Around line 285-296: The import fallback in the adapter is using an internal
module path for the trainer API, but `SentenceTransformerTrainer`,
`SentenceTransformerTrainingArguments`, and `losses` should be imported directly
from the public `sentence_transformers` package. Update the import block in
`adaptation.adapter` to remove the `sentence_transformers.sentence_transformer`
branch and keep only the public-package import path for these symbols.
In `@tritopic/adaptation/config.py`:
- Around line 14-49: Add validation to AdaptationConfig, ideally in a
__post_init__ method, to fail fast on invalid ranges instead of silently
producing empty train/holdout splits. Check key fields like holdout_frac,
entropy_top_frac, n_triplets, and related numeric settings for sensible bounds,
and raise clear errors when values are out of range. Keep the validation close
to the AdaptationConfig dataclass so adapt_and_refit and downstream callers
always receive a valid configuration.
In `@tritopic/adaptation/triplets.py`:
- Around line 128-164: The same nearest-neighbor same/different triplet
extraction logic is duplicated in `_nearest_same_diff`, matching
`_sample_triplets_fast` in `llm_granularity.py` and its
`_compat.sample_triplets_fast` export. Refactor the shared
normalization/NearestNeighbors/inner selection loop into a single helper in
`llm_granularity.py`, then reuse or re-export it through `_compat.py` so
`_nearest_same_diff` can call the shared implementation instead of maintaining a
parallel copy.
🪄 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: 76fdad15-198f-4a91-8d67-1ad536d6cbe9
📒 Files selected for processing (28)
.github/workflows/ci.yml.gitignoreREADME.mdbenchmarks/adaptation_quality_report.pynotebooks/cumulative_tritopic_benchmark.ipynbnotebooks/embedding_adaptation_demo.ipynbpyproject.tomlrun_benchmark.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_quote_verification.pytests/test_report_themes.pytritopic/adaptation/__init__.pytritopic/adaptation/_compat.pytritopic/adaptation/adapter.pytritopic/adaptation/config.pytritopic/adaptation/correction.pytritopic/adaptation/evaluation.pytritopic/adaptation/keyphrase.pytritopic/adaptation/pipeline.pytritopic/adaptation/triplets.pytritopic/core/model.pytritopic/utils/__init__.pytritopic/utils/quote_verification.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🧹 Nitpick comments (10)
tritopic/adaptation/adapter.py (1)
285-296: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport the trainer API from the public package
SentenceTransformerTrainer,SentenceTransformerTrainingArguments, andlossesare re-exported bysentence_transformers; thesentence_transformers.sentence_transformerimport path can be removed.🤖 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/adaptation/adapter.py` around lines 285 - 296, The import fallback in the adapter is using an internal module path for the trainer API, but `SentenceTransformerTrainer`, `SentenceTransformerTrainingArguments`, and `losses` should be imported directly from the public `sentence_transformers` package. Update the import block in `adaptation.adapter` to remove the `sentence_transformers.sentence_transformer` branch and keep only the public-package import path for these symbols.README.md (1)
103-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInstall comment omits
torchdependency.
pyproject.toml's newadaptationextra pulls indatasets,accelerate, andtorch>=2.0.0, but this comment only mentions the first two —torchis a substantial download worth calling out.📝 Proposed fix
-# With LLM-guided embedding fine-tuning (adds datasets, accelerate) +# With LLM-guided embedding fine-tuning (adds datasets, accelerate, torch) pip install tritopic[adaptation]🤖 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 103 - 105, The installation note for the tritopic[adaptation] extra is missing the torch dependency, so update the README comment near the adaptation install command to mention that this extra also installs torch>=2.0.0 alongside datasets and accelerate. Keep the wording aligned with the existing install guidance and make sure the README matches the dependencies declared in pyproject.toml for the adaptation extra.benchmarks/adaptation_quality_report.py (3)
116-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused
new_modelbinding (Ruff RUF059).🧹 Proposed fix
- new_model, report = adapt_and_refit( + _new_model, report = adapt_and_refit( model, labeler, config=_adapt_cfg(), evaluate=True, labels_true=labels_true )🤖 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` at line 116, The `new_model` result from `adapt_and_refit` is not used, triggering Ruff RUF059. Update the assignment in the adaptation quality report flow to ignore that return value explicitly, and keep only the `report` binding at the call site in `adaptation_quality_report.py` so the intent is clear and the unused variable warning is removed.Source: Linters/SAST tools
162-171: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant full-dataset fetch just to read category names.
fetch_20newsgroups(subset="all")is called once (line 165) purely to index.target_names, then again (line 163) with thecategoriesfilter — loading/parsing the full ~18K-document dataset twice. 20 Newsgroups'target_namesare stable; consider fetching once and slicing, or hardcoding the category name list.⚡ Proposed fix
- cats = [0, 1, 2, 3, 4] # 5 categories, kept small for a fast local run - data = fetch_20newsgroups( - subset="all", - categories=[fetch_20newsgroups(subset="all").target_names[c] for c in cats], - remove=("headers", "footers", "quotes"), - ) + cats = [0, 1, 2, 3, 4] # 5 categories, kept small for a fast local run + all_target_names = fetch_20newsgroups(subset="all").target_names + data = fetch_20newsgroups( + subset="all", + categories=[all_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 162 - 171, Avoid calling fetch_20newsgroups(subset="all") twice in the same setup block; the second call is only being used to read .target_names. In adaptation_quality_report.py, update the data-loading logic around the cats list and fetch_20newsgroups call so the category names are obtained without reloading the full dataset a second time, either by fetching once and reusing target_names or by using a fixed category-name list before the filtered fetch.
50-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
OracleLabelerhere duplicates the identical class defined innotebooks/embedding_adaptation_demo.ipynb. Consider extracting a shared test-utility (e.g. undertritopic.adaptationtest helpers) both can import, since the regex depends on the internal triplet-prompt format and any drift would silently break both.🤖 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 50 - 75, The OracleLabeler implementation is duplicated here and in the notebook, so factor the shared logic into a common test utility and import it from both places. Move the triplet-parsing and snippet-to-label matching behavior from OracleLabeler into a reusable helper under the adaptation test helpers, then have this benchmark code use that shared symbol so the regex and prompt-format assumptions stay centralized. Keep the existing OracleLabeler entry point but make it delegate to the shared helper to avoid drift.notebooks/embedding_adaptation_demo.ipynb (1)
108-140: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSilent misconfiguration when
LABEL_COLis set but missing.If a user sets
LABEL_COLto a typo'd or wrong column name,LABEL_COL and LABEL_COL in raw.columnssilently evaluates false andtrue_labelsbecomesNone— Section 3's oracle self-test is then silently skipped with the generic "no ground-truth" message, masking a configuration typo.💡 Proposed fix
- if LABEL_COL and LABEL_COL in raw.columns: + if LABEL_COL and LABEL_COL in raw.columns: codes, categories_index = pd.factorize(raw[LABEL_COL]) true_labels = codes categories = categories_index.tolist() + elif LABEL_COL: + raise ValueError(f"LABEL_COL={LABEL_COL!r} not found in columns: {list(raw.columns)}") else: true_labels = None categories = None🤖 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/embedding_adaptation_demo.ipynb` around lines 108 - 140, The CSV loading block in the notebook silently treats a configured LABEL_COL as “not provided” when the column name is wrong, which hides typos. Update the logic around raw, true_labels, and categories so that if LABEL_COL is set but not present in raw.columns, it raises a clear error instead of falling through to the “no ground-truth” path. Keep the existing behavior only when LABEL_COL is unset, and preserve the current factorize-based label handling when the column exists.pyproject.toml (1)
96-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
adaptationinfull.datasetsandaccelerateare pinned twice here; referencingtritopic[adaptation]would keep the extras in sync.🤖 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` around lines 96 - 115, The full extra duplicates the datasets and accelerate pins already defined in adaptation, so keep them in sync by reusing the adaptation extra from the full extra instead of repeating those dependencies. Update the full extras block in pyproject.toml so the shared Trainer-related requirements are sourced through tritopic[adaptation], while preserving the other full-only packages like anthropic, openai, and torch.tritopic/adaptation/_compat.py (1)
30-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSort
__all__per Ruff RUF022.♻️ Suggested fix
__all__ = [ - "build_triplet_prompt", - "TRIPLET_SCHEMA", - "parse_triplet_response", - "sample_triplets_fast", - "sample_triplets_informed", + "TRIPLET_SCHEMA", + "build_triplet_prompt", + "parse_triplet_response", + "sample_triplets_fast", + "sample_triplets_informed", ]🤖 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/adaptation/_compat.py` around lines 30 - 36, The __all__ export list in _compat.py is not sorted, which triggers Ruff RUF022. Reorder the entries in __all__ alphabetically while keeping the same exported symbols, and leave the list structure intact so the module exports remain unchanged.Source: Linters/SAST tools
tritopic/adaptation/triplets.py (1)
128-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate k-NN same/diff extraction logic.
_nearest_same_diffreimplements the exact same same-cluster/diff-cluster neighbour extraction loop as_sample_triplets_fastintritopic/labeling/llm_granularity.py(re-exported via_compat.sample_triplets_fast) — same normalization, sameNearestNeighborscall, same inner loop. Consider factoring the shared loop out of_sample_triplets_fastinllm_granularity.pyand re-exporting it through_compat.py, consistent with the detachability boundary this package already establishes.🤖 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/adaptation/triplets.py` around lines 128 - 164, The same nearest-neighbor same/different triplet extraction logic is duplicated in `_nearest_same_diff`, matching `_sample_triplets_fast` in `llm_granularity.py` and its `_compat.sample_triplets_fast` export. Refactor the shared normalization/NearestNeighbors/inner selection loop into a single helper in `llm_granularity.py`, then reuse or re-export it through `_compat.py` so `_nearest_same_diff` can call the shared implementation instead of maintaining a parallel copy.tritopic/adaptation/config.py (1)
14-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider validating config ranges in
__post_init__.Fields like
holdout_frac,entropy_top_frac, andn_tripletshave no bounds checking. E.g.holdout_frac >= 1.0would push every judgment into the holdout split (empty train set) orholdout_frac < 0would push everything into train (empty holdout) — both silently degradeadapt_and_refit's evaluation rather than failing fast with a clear error.♻️ Suggested validation
+ def __post_init__(self) -> None: + if not 0.0 <= self.holdout_frac < 1.0: + raise ValueError(f"holdout_frac must be in [0, 1), got {self.holdout_frac}") + if not 0.0 < self.entropy_top_frac <= 1.0: + raise ValueError(f"entropy_top_frac must be in (0, 1], got {self.entropy_top_frac}") + if self.n_triplets <= 0: + raise ValueError(f"n_triplets must be positive, got {self.n_triplets}")🤖 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/adaptation/config.py` around lines 14 - 49, Add validation to AdaptationConfig, ideally in a __post_init__ method, to fail fast on invalid ranges instead of silently producing empty train/holdout splits. Check key fields like holdout_frac, entropy_top_frac, n_triplets, and related numeric settings for sensible bounds, and raise clear errors when values are out of range. Keep the validation close to the AdaptationConfig dataclass so adapt_and_refit and downstream callers always receive a valid configuration.
🤖 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:
- Around line 39-40: The CI test command is over-excluding by ignoring the
entire 20NG integration module, which also drops the CI-safe ng20_small
coverage. Update the workflow’s test step so the pytest call on
test_integration_20ng keeps the module included and only filters out slow cases,
using the existing test command pattern in the CI job rather than --ignore on
the whole file.
In `@tritopic/adaptation/keyphrase.py`:
- Around line 147-152: The concat_encode branch in keyphrase adaptation ignores
the normalize option, unlike the average branch. Update the concat_encode path
in the keyphrase encoding flow to apply the same normalization behavior when
normalize=True, likely by normalizing the result after encoder.encode before
returning it, so both branches in the encode logic behave consistently.
In `@tritopic/adaptation/triplets.py`:
- Around line 265-330: The cache in the triplet adaptation flow reuses judgments
based only on content hash, so cached raw row indices can be replayed against a
reordered or partially different corpus. Update the caching path in the triplet
judgment logic around _load_cache_index, _append_cache, and the
resolved/judgment construction to store a corpus fingerprint or stable document
IDs alongside each TripletJudgment, then verify that fingerprint before
accepting a cache hit and invalidate or skip mismatched entries.
In `@tritopic/core/model.py`:
- Around line 1986-1999: The adaptation path in the model update flow leaves the
fitted object with the original encoder, so post-adaptation inference can embed
new documents in the wrong space. In the method that calls adapt_and_refit() and
then does self.__dict__.update(new_model.__dict__), make sure the adapted
encoder from new_model is persisted onto the returned model as well, especially
the _embedding_engine used by transform() and transform_proba(). Verify the
updated model keeps the adapted embedding engine consistently with the adapted
centroids/distance state.
---
Nitpick comments:
In `@benchmarks/adaptation_quality_report.py`:
- Line 116: The `new_model` result from `adapt_and_refit` is not used,
triggering Ruff RUF059. Update the assignment in the adaptation quality report
flow to ignore that return value explicitly, and keep only the `report` binding
at the call site in `adaptation_quality_report.py` so the intent is clear and
the unused variable warning is removed.
- Around line 162-171: Avoid calling fetch_20newsgroups(subset="all") twice in
the same setup block; the second call is only being used to read .target_names.
In adaptation_quality_report.py, update the data-loading logic around the cats
list and fetch_20newsgroups call so the category names are obtained without
reloading the full dataset a second time, either by fetching once and reusing
target_names or by using a fixed category-name list before the filtered fetch.
- Around line 50-75: The OracleLabeler implementation is duplicated here and in
the notebook, so factor the shared logic into a common test utility and import
it from both places. Move the triplet-parsing and snippet-to-label matching
behavior from OracleLabeler into a reusable helper under the adaptation test
helpers, then have this benchmark code use that shared symbol so the regex and
prompt-format assumptions stay centralized. Keep the existing OracleLabeler
entry point but make it delegate to the shared helper to avoid drift.
In `@notebooks/embedding_adaptation_demo.ipynb`:
- Around line 108-140: The CSV loading block in the notebook silently treats a
configured LABEL_COL as “not provided” when the column name is wrong, which
hides typos. Update the logic around raw, true_labels, and categories so that if
LABEL_COL is set but not present in raw.columns, it raises a clear error instead
of falling through to the “no ground-truth” path. Keep the existing behavior
only when LABEL_COL is unset, and preserve the current factorize-based label
handling when the column exists.
In `@pyproject.toml`:
- Around line 96-115: The full extra duplicates the datasets and accelerate pins
already defined in adaptation, so keep them in sync by reusing the adaptation
extra from the full extra instead of repeating those dependencies. Update the
full extras block in pyproject.toml so the shared Trainer-related requirements
are sourced through tritopic[adaptation], while preserving the other full-only
packages like anthropic, openai, and torch.
In `@README.md`:
- Around line 103-105: The installation note for the tritopic[adaptation] extra
is missing the torch dependency, so update the README comment near the
adaptation install command to mention that this extra also installs torch>=2.0.0
alongside datasets and accelerate. Keep the wording aligned with the existing
install guidance and make sure the README matches the dependencies declared in
pyproject.toml for the adaptation extra.
In `@tritopic/adaptation/_compat.py`:
- Around line 30-36: The __all__ export list in _compat.py is not sorted, which
triggers Ruff RUF022. Reorder the entries in __all__ alphabetically while
keeping the same exported symbols, and leave the list structure intact so the
module exports remain unchanged.
In `@tritopic/adaptation/adapter.py`:
- Around line 285-296: The import fallback in the adapter is using an internal
module path for the trainer API, but `SentenceTransformerTrainer`,
`SentenceTransformerTrainingArguments`, and `losses` should be imported directly
from the public `sentence_transformers` package. Update the import block in
`adaptation.adapter` to remove the `sentence_transformers.sentence_transformer`
branch and keep only the public-package import path for these symbols.
In `@tritopic/adaptation/config.py`:
- Around line 14-49: Add validation to AdaptationConfig, ideally in a
__post_init__ method, to fail fast on invalid ranges instead of silently
producing empty train/holdout splits. Check key fields like holdout_frac,
entropy_top_frac, n_triplets, and related numeric settings for sensible bounds,
and raise clear errors when values are out of range. Keep the validation close
to the AdaptationConfig dataclass so adapt_and_refit and downstream callers
always receive a valid configuration.
In `@tritopic/adaptation/triplets.py`:
- Around line 128-164: The same nearest-neighbor same/different triplet
extraction logic is duplicated in `_nearest_same_diff`, matching
`_sample_triplets_fast` in `llm_granularity.py` and its
`_compat.sample_triplets_fast` export. Refactor the shared
normalization/NearestNeighbors/inner selection loop into a single helper in
`llm_granularity.py`, then reuse or re-export it through `_compat.py` so
`_nearest_same_diff` can call the shared implementation instead of maintaining a
parallel copy.
🪄 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: 76fdad15-198f-4a91-8d67-1ad536d6cbe9
📒 Files selected for processing (28)
.github/workflows/ci.yml.gitignoreREADME.mdbenchmarks/adaptation_quality_report.pynotebooks/cumulative_tritopic_benchmark.ipynbnotebooks/embedding_adaptation_demo.ipynbpyproject.tomlrun_benchmark.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_quote_verification.pytests/test_report_themes.pytritopic/adaptation/__init__.pytritopic/adaptation/_compat.pytritopic/adaptation/adapter.pytritopic/adaptation/config.pytritopic/adaptation/correction.pytritopic/adaptation/evaluation.pytritopic/adaptation/keyphrase.pytritopic/adaptation/pipeline.pytritopic/adaptation/triplets.pytritopic/core/model.pytritopic/utils/__init__.pytritopic/utils/quote_verification.py
🛑 Comments failed to post (4)
.github/workflows/ci.yml (1)
39-40: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Check whether fast tests in this file require network access at import/collection time rg -n "`@pytest.mark.slow`|fetch_20newsgroups|def test_" tests/test_integration_20ng.pyRepository: nevil-mathew/topic-extraction-poc
Length of output: 1344
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the test module around the fixtures and docstring sed -n '1,120p' tests/test_integration_20ng.pyRepository: nevil-mathew/topic-extraction-poc
Length of output: 4727
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the rest of the test module for any offline/data-cache handling sed -n '120,340p' tests/test_integration_20ng.py # Check the CI workflow step and surrounding context sed -n '1,120p' .github/workflows/ci.ymlRepository: nevil-mathew/topic-extraction-poc
Length of output: 10342
🏁 Script executed:
#!/bin/bash set -euo pipefail # Search for any explicit caching/offline setup for fetch_20newsgroups rg -n "fetch_20newsgroups|download_if_missing|data_home|20newsgroups|cache" .github tests tritopicRepository: nevil-mathew/topic-extraction-poc
Length of output: 8698
Don’t ignore the whole 20NG test module — this skips the CI-safe 2k-doc tests in
ng20_smalltoo. If only the slow 6k-doc path is the problem, runpytest tests/test_integration_20ng.py -m "not slow"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 @.github/workflows/ci.yml around lines 39 - 40, The CI test command is over-excluding by ignoring the entire 20NG integration module, which also drops the CI-safe ng20_small coverage. Update the workflow’s test step so the pytest call on test_integration_20ng keeps the module included and only filters out slow cases, using the existing test command pattern in the CI job rather than --ignore on the whole file.tritopic/adaptation/keyphrase.py (1)
147-152: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
concat_encodeignoresnormalize.The
averagebranch honorsnormalize(l2-normalizing at the end), butconcat_encodereturns the raw encoder output regardless ofnormalize=True(the default). This inconsistency can surprise callers relying on unit-length vectors for cosine comparisons.Proposed fix
if mode == "concat_encode": texts = [ doc + ("\nKeyphrases: " + ", ".join(kws) if kws else "") for doc, kws in zip(documents, keyphrases) ] - return encoder.encode(texts) + out = np.asarray(encoder.encode(texts), dtype=np.float64) + if not normalize: + return out + norms = np.linalg.norm(out, axis=1, keepdims=True) + norms = np.where(norms == 0, 1.0, norms) + return out / norms📝 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.if mode == "concat_encode": texts = [ doc + ("\nKeyphrases: " + ", ".join(kws) if kws else "") for doc, kws in zip(documents, keyphrases) ] out = np.asarray(encoder.encode(texts), dtype=np.float64) if not normalize: return out norms = np.linalg.norm(out, axis=1, keepdims=True) norms = np.where(norms == 0, 1.0, norms) return out / norms🤖 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/adaptation/keyphrase.py` around lines 147 - 152, The concat_encode branch in keyphrase adaptation ignores the normalize option, unlike the average branch. Update the concat_encode path in the keyphrase encoding flow to apply the same normalization behavior when normalize=True, likely by normalizing the result after encoder.encode before returning it, so both branches in the encode logic behave consistently.tritopic/adaptation/triplets.py (1)
265-330: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant file sections with line numbers. file="tritopic/adaptation/triplets.py" wc -l "$file" sed -n '150,380p' "$file" | cat -nRepository: nevil-mathew/topic-extraction-poc
Length of output: 11083
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate cache-related symbols and TripletJudgment definition/serialization paths. rg -n "TripletJudgment|_load_cache_index|_append_cache|_content_hash|cache_path|cache_index|raw_answer|split" tritopic -SRepository: nevil-mathew/topic-extraction-poc
Length of output: 7024
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read the remaining methods and any serialization helpers near the end of the file. sed -n '380,430p' tritopic/adaptation/triplets.py | cat -nRepository: nevil-mathew/topic-extraction-poc
Length of output: 1780
🏁 Script executed:
#!/bin/bash set -euo pipefail # Look for documentation about the triplet cache and any assumptions about document order. rg -n "survive resampling/reordering|cache|TripletBank|TripletJudgment|anchor" tritopic -g '!*.pyc' -SRepository: nevil-mathew/topic-extraction-poc
Length of output: 16549
🏁 Script executed:
#!/bin/bash set -euo pipefail # Probe whether the cached anchor/positive/negative indices are later interpreted against the current documents list. python3 - <<'PY' from pathlib import Path import re text = Path("tritopic/adaptation/triplets.py").read_text() for name in ["collect(", "to_training_texts(", "_load_cache_index(", "_append_cache(", "save(", "load("]: print(f"\n== {name} ==") m = re.search(rf"^.*{re.escape(name)}.*$", text, re.M) if m: start = max(0, m.start() - 250) end = min(len(text), m.end() + 1500) print(text[start:end]) PYRepository: nevil-mathew/topic-extraction-poc
Length of output: 9358
Cached judgments need a corpus fingerprint
anchor/positive/negativeare cached as raw row indices, but cache hits are keyed only by content hash and replay those indices verbatim. Reusing the cache on a reordered or different-but-overlapping corpus will silently point at the wrong documents. Store a corpus fingerprint or stable document IDs with the cache and invalidate on mismatch.🤖 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/adaptation/triplets.py` around lines 265 - 330, The cache in the triplet adaptation flow reuses judgments based only on content hash, so cached raw row indices can be replayed against a reordered or partially different corpus. Update the caching path in the triplet judgment logic around _load_cache_index, _append_cache, and the resolved/judgment construction to store a corpus fingerprint or stable document IDs alongside each TripletJudgment, then verify that fingerprint before accepting a cache hit and invalidate or skip mismatched entries.tritopic/core/model.py (1)
1986-1999: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Confirm transform() relies on the base embedding engine and no adapter is stored on the model. rg -nP -C3 '_embedding_engine\.encode' tritopic/core/model.py rg -nP '(linear_|adapter|EmbeddingAdapter|adapted_encoder)' tritopic/core/model.pyRepository: nevil-mathew/topic-extraction-poc
Length of output: 1642
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== model.py outline ==" ast-grep outline tritopic/core/model.py --view expanded | sed -n '1,260p' echo echo "== adapt_and_refit references ==" rg -n -C4 'def adapt_and_refit|adapt_and_refit\(' tritopic/core/model.py tritopic -g '*.py' echo echo "== state related assignments near adaptation ==" sed -n '1910,2005p' tritopic/core/model.pyRepository: nevil-mathew/topic-extraction-poc
Length of output: 11784
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== tritopic/adaptation/pipeline.py outline ==" ast-grep outline tritopic/adaptation/pipeline.py --view expanded | sed -n '1,260p' echo echo "== adapt_and_refit body ==" sed -n '1,240p' tritopic/adaptation/pipeline.py echo echo "== adaptation engine references in repo ==" rg -n -C3 'LinearAdapter|_embedding_engine|adapted embeddings|fine-tune|sentence-transformers|encoder' tritopic/adaptation tritopic/core -g '*.py'Repository: nevil-mathew/topic-extraction-poc
Length of output: 33622
Persist the adapted encoder, or
transform()will mix embedding spaces
adapt_and_refit()refitsnew_modelfromnew_embeddings, butnew_model._embedding_engineis still the original base encoder. Afterself.__dict__.update(new_model.__dict__),transform()/transform_proba()on unseen docs will embed in the base space and compare against adapted-space centroids, producing wrong assignments. Existing fitted outputs are fine; this only affects post-adaptation inference on new documents.🤖 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/model.py` around lines 1986 - 1999, The adaptation path in the model update flow leaves the fitted object with the original encoder, so post-adaptation inference can embed new documents in the wrong space. In the method that calls adapt_and_refit() and then does self.__dict__.update(new_model.__dict__), make sure the adapted encoder from new_model is persisted onto the returned model as well, especially the _embedding_engine used by transform() and transform_proba(). Verify the updated model keeps the adapted embedding engine consistently with the adapted centroids/distance state.
This was referenced Jul 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
EmbeddingAdapter, evaluation harness, keyphrase/correction extras,
adapt_and_refit() pipeline
tune_resolution_with_llm's ergonomics
(synthetic + real 20 Newsgroups scenarios)
notebook with inline assertions, real 20NG data
adaptationextra (datasets, accelerate)with two-stage/bias-mitigation details and a diagnostics example
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Summary by CodeRabbit