fix(core): only show a collision summary when it actually disambiguates#225
Merged
Merged
Conversation
Two distinct sessions with the same first prompt get the same ai-title from the CLI, so their sidebar rows collide *and* share a summary. The disambiguator surfaced that summary anyway — its condition was merely that the shown title diverges from the summary, which is true here — so both rows repeated identical text and the age fallback was never reached. Gate the summary on it being unique among the rows sharing that title, and fold colliding_titles + collision_subtitle into one per-group pass (session_disambiguators) since the uniqueness check needs the whole group. Closes #220
guillaumejay
force-pushed
the
fix/disambiguator-shared-summary-v2
branch
from
July 16, 2026 19:50
886c03b to
1a91cb7
Compare
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.
Closes #220. Supersedes #221, which was cut from a stale main and conflicted irreconcilably once
core/src/app.rswas split into submodules (#169/#173).Problem
Two sidebar rows for two genuinely distinct sessions read identically apart from the counter:
The same question asked twice in one project makes the CLI derive the same
ai-titlefor both, so the titles collide and the summaries match.collision_subtitlesurfaced the summary regardless — its only condition was that the shown title diverges from the summary, which holds here — so both rows repeated the same text and the relative-age fallback was never reached.Fix
The summary is shown only when it is unique among the rows sharing that title; otherwise the row falls back to the age, which always differs.
Since that uniqueness check needs the whole group,
colliding_titlesandcollision_subtitleare folded into a single per-group pass,App::session_disambiguators(group) -> HashMap<String, Option<String>>: presence marks a colliding row, the value is the separating summary orNonefor the age fallback. One traversal instead of two, and the two concepts no longer have to be kept in sync at the call site. Both had exactly one caller.Uniqueness is checked on the
(title, summary)pair rather than the summary alone: a row with a different title that happens to share a summary must not make the colliding pair look ambiguous. A test pins that.Tests
the_same_question_asked_twice_falls_back_to_the_age— the reported case, plus the third-row guard above./cleartitle-carryover case (divergent, unique summaries) still surfaces the summary.Verification
cargo fmt --all --checkandcargo test --workspacegreen.cargo clippy --workspacecurrently fails on Windows for an unrelated pre-existing reason —crates/pty/src/status.rs, fixed in #224; with that applied, clippy is green here too. The touched crates (termherd-core,termherd-app, 437 tests) are clean.Not eyeballed in a running build — the change is core logic with unit coverage; the view edit is a mechanical swap of the same two values.