fix(report): stop LLM title clustering from over-merging unrelated sessions - #755
Merged
Merged
Conversation
…ssions
The Rust title-clustering fallback (used by apple-fm and other non-LLM
backends) collapsed unrelated sessions into one giant group.
Three root causes, fixed here:
(a) Over-merge via absolute token count + union growth. `tokens_overlap`
returned true whenever two titles shared >= 2 significant tokens,
regardless of set size, and `cluster_titles` grew each cluster's
signature to the UNION of all member tokens. Together a cluster could
transitively absorb unrelated sessions that merely shared a couple of
incidental words. Now overlap is the OVERLAP COEFFICIENT
(shared / min(|a|,|b|) >= 0.6), and clusters keep per-member token
sets — a candidate joins only if it overlaps an actual member, never an
accumulated union — so genuine near-duplicates still merge but
incidental two-token overlaps no longer chain.
(b) Stopword-only titles lumped together. Titles that reduce to no
significant tokens were all forced into one arbitrary group. Each
distinct normalized title now gets its own group; identical
stopword-only titles still collapse.
(c) ASCII-only case folding. `to_ascii_lowercase` left non-ASCII capitals
untouched while tokens kept Unicode, so "Café"/"café" clustered apart.
Now uses full `to_lowercase()`.
Adds regression tests for each case; existing near-duplicate / unrelated /
exact-duplicate tests still pass.
Confidence: high
Scope-risk: narrow
Not-tested: interaction with the sibling report-date PR (only the
clustering functions were touched)
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89643a06eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let union = a.len() + b.len() - shared; | ||
| union > 0 && (shared as f64 / union as f64) >= 0.6 | ||
| let smaller = a.len().min(b.len()); | ||
| smaller > 0 && (shared as f64 / smaller as f64) >= CLUSTER_SIMILARITY_THRESHOLD |
There was a problem hiding this comment.
With the new overlap coefficient, any title that reduces to a single significant token gets a perfect score against every longer title containing that token, so a generic summary like Fix API/API will cluster with unrelated Add API auth, Update API billing, etc. This regresses the over-merge behavior for apple-fm/non-LLM grouping because the previous Jaccard/shared-token rule would not merge on just one shared token; singleton titles need a stricter rule such as requiring at least two significant shared tokens.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 715c070. tokens_overlap now requires at least two shared significant tokens when the smaller token set has only one token (if smaller <= 1 { return shared >= 2; }), before applying the overlap coefficient. This prevents a singleton title ("API"/"Fix API") from merging with every unrelated longer title containing that token, restoring the stricter behavior for apple-fm/non-LLM grouping while keeping genuine multi-token near-duplicates clustering. Regression tests added (tokens_overlap_singleton_does_not_overcluster, cluster_titles_does_not_overcluster_singletons). cargo test + clippy green.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Lowercase report clustering titles before filtering token characters and drop Unicode combining marks introduced by lowercase expansion. This keeps case-only variants such as Turkish dotted I clustered correctly while preserving the overlap-coefficient behavior from the PR. Also converts three report test fixtures from Vec to arrays so clippy remains clean. Constraint: Only report.rs was in scope for PR validation Rejected: Add a Unicode normalization dependency | new dependencies were out of scope for this PR fix Confidence: high Scope-risk: narrow Tested: cargo test -p tokscale-cli commands::report::tests Tested: cargo test -p tokscale-cli -- --skip antigravity::tests::identity_probe_request_decodes_chunked_antigravity_response --skip antigravity::tests::identity_probe_request_prefers_chunked_over_content_length --skip antigravity::tests::identity_probe_request_uses_probe_cap_for_large_bodies --skip antigravity::tests::rpc_request_rejects_oversized_content_length_body --skip antigravity::tests::read_chunked_body_rejects_oversized_accumulated_chunks Tested: cargo clippy -p tokscale-cli --tests -- -D warnings Not-tested: Full unfiltered cargo test in this sandbox; antigravity TCP listener tests fail because loopback bind returns Operation not permitted
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…tles Address automated review feedback on the title-clustering logic. The overlap coefficient `shared / min(|a|, |b|)` scores a perfect 1.0 for a single-token title against any longer title that merely contains that token, so a generic summary like "API"/"Fix API" clustered with every unrelated "Add API auth"/"Update API billing". Require at least two shared tokens when the smaller set has only one token; genuinely-related multi-token titles still cluster. Combining-mark stripping was applied asymmetrically: an NFC title kept its precomposed letter while an NFD equivalent had its mark stripped, tokenizing identical titles differently and splitting them across clusters. Normalize to NFC before lowercasing/stripping in both significant_tokens and normalized_title so canonically-equivalent titles converge. Constraint: must not regress the original over-merge fix this PR introduced Rejected: fall back to Jaccard for tiny sets | shared>=2 guard is simpler and matches reviewer suggestion Confidence: high Scope-risk: narrow
t1000040
pushed a commit
to tmobi-internal/tokscale
that referenced
this pull request
Jun 30, 2026
…ssions (junhoyeo#755) * fix(report): stop LLM title clustering from over-merging unrelated sessions The Rust title-clustering fallback (used by apple-fm and other non-LLM backends) collapsed unrelated sessions into one giant group. Three root causes, fixed here: (a) Over-merge via absolute token count + union growth. `tokens_overlap` returned true whenever two titles shared >= 2 significant tokens, regardless of set size, and `cluster_titles` grew each cluster's signature to the UNION of all member tokens. Together a cluster could transitively absorb unrelated sessions that merely shared a couple of incidental words. Now overlap is the OVERLAP COEFFICIENT (shared / min(|a|,|b|) >= 0.6), and clusters keep per-member token sets — a candidate joins only if it overlaps an actual member, never an accumulated union — so genuine near-duplicates still merge but incidental two-token overlaps no longer chain. (b) Stopword-only titles lumped together. Titles that reduce to no significant tokens were all forced into one arbitrary group. Each distinct normalized title now gets its own group; identical stopword-only titles still collapse. (c) ASCII-only case folding. `to_ascii_lowercase` left non-ASCII capitals untouched while tokens kept Unicode, so "Café"/"café" clustered apart. Now uses full `to_lowercase()`. Adds regression tests for each case; existing near-duplicate / unrelated / exact-duplicate tests still pass. Confidence: high Scope-risk: narrow Not-tested: interaction with the sibling report-date PR (only the clustering functions were touched) * fix(report): normalize Unicode title tokens Lowercase report clustering titles before filtering token characters and drop Unicode combining marks introduced by lowercase expansion. This keeps case-only variants such as Turkish dotted I clustered correctly while preserving the overlap-coefficient behavior from the PR. Also converts three report test fixtures from Vec to arrays so clippy remains clean. Constraint: Only report.rs was in scope for PR validation Rejected: Add a Unicode normalization dependency | new dependencies were out of scope for this PR fix Confidence: high Scope-risk: narrow Tested: cargo test -p tokscale-cli commands::report::tests Tested: cargo test -p tokscale-cli -- --skip antigravity::tests::identity_probe_request_decodes_chunked_antigravity_response --skip antigravity::tests::identity_probe_request_prefers_chunked_over_content_length --skip antigravity::tests::identity_probe_request_uses_probe_cap_for_large_bodies --skip antigravity::tests::rpc_request_rejects_oversized_content_length_body --skip antigravity::tests::read_chunked_body_rejects_oversized_accumulated_chunks Tested: cargo clippy -p tokscale-cli --tests -- -D warnings Not-tested: Full unfiltered cargo test in this sandbox; antigravity TCP listener tests fail because loopback bind returns Operation not permitted * fix(report): guard singleton over-clustering and normalize NFC/NFD titles Address automated review feedback on the title-clustering logic. The overlap coefficient `shared / min(|a|, |b|)` scores a perfect 1.0 for a single-token title against any longer title that merely contains that token, so a generic summary like "API"/"Fix API" clustered with every unrelated "Add API auth"/"Update API billing". Require at least two shared tokens when the smaller set has only one token; genuinely-related multi-token titles still cluster. Combining-mark stripping was applied asymmetrically: an NFC title kept its precomposed letter while an NFD equivalent had its mark stripped, tokenizing identical titles differently and splitting them across clusters. Normalize to NFC before lowercasing/stripping in both significant_tokens and normalized_title so canonically-equivalent titles converge. Constraint: must not regress the original over-merge fix this PR introduced Rejected: fall back to Jaccard for tiny sets | shared>=2 guard is simpler and matches reviewer suggestion Confidence: high Scope-risk: narrow
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
1 participant
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.
Problem
The Rust title-clustering fallback in
crates/tokscale-cli/src/commands/report.rs(used by apple-fm and other non-LLM backends, PR #726) over-merges: unrelated sessions collapse into one giant group.Three root causes:
tokens_overlapreturnedtruewhenever two titles shared>= 2significant tokens regardless of set size, andcluster_titlesgrew each cluster signature to the union of all member tokens. A cluster could therefore transitively absorb unrelated sessions that merely shared a couple of incidental words.to_ascii_lowercaseleft non-ASCII capitals untouched while tokens kept Unicode, soCafé/caféclustered apart.Fix
shared >= 2rule with the overlap coefficientshared / min(|a|, |b|) >= 0.6(CLUSTER_SIMILARITY_THRESHOLD). Clusters now keep per-member token sets instead of a grown union; a candidate joins only if it overlaps an actual member. Genuine near-duplicates still merge (a short title fully contained in a long one scores 1.0), but incidental two-token overlaps no longer chain unrelated titles.normalized_title): each distinct stopword-only title gets its own group, identical ones still collapse.to_lowercase()for both tokenization and normalization.Only the clustering functions were touched (a sibling report-date PR also edits this file).
Tests
Added regression tests that fail without the fix:
tokens_overlap_uses_ratio_not_absolute_countcluster_titles_does_not_transitively_absorb_unrelatedcluster_titles_separates_distinct_stopword_only_titlessignificant_tokens_folds_non_ascii_casecluster_titles_merges_non_ascii_case_variantsAll existing clustering tests (near-duplicate / unrelated / exact-duplicate / label) still pass.
cargo test -p tokscale-cligreen (122 + unit tests);cargo clippy -p tokscale-cli --testsintroduces no new warnings (3 pre-existinguseless_vecwarnings on main remain untouched).Residual concern
Interaction with the sibling report-date PR was not tested (only the clustering functions were modified, so conflict risk is low).
🤖 Generated with Claude Code
Summary by cubic
Fixes over-merging in the Rust title-clustering fallback and unifies Unicode titles so unrelated sessions don’t collapse together. Adds NFC normalization and a singleton-token guard so generic one-word titles (e.g., “API”) don’t over-cluster.
Bug Fixes
Dependencies
unicode-normalizationfor NFC normalization.Written for commit 715c070. Summary will update on new commits.