Skip to content

Web UI: do not rewrite the typed word when accepting an equal suggestion#109106

Merged
alexey-milovidov merged 2 commits into
masterfrom
play-suggestion-noop
Jul 2, 2026
Merged

Web UI: do not rewrite the typed word when accepting an equal suggestion#109106
alexey-milovidov merged 2 commits into
masterfrom
play-suggestion-noop

Conversation

@alexey-milovidov

@alexey-milovidov alexey-milovidov commented Jul 2, 2026

Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Web UI (Play): when the highlighted completion is the same word that is already typed, just in a different case (e.g. HASH offered for a typed hash), pressing Right/Tab/Enter no longer rewrites the word to the canonical spelling — it is now a no-op.

Details

In programs/server/play.html, the suggestion popup keeps an "empty completion" for the word you have already typed so you can see it is recognized. That candidate is selected first and renders as empty text (nothing visible on the cursor line), while real extensions are shown below.

Accepting a completion (Right / Tab / Enter, or click) replaces the typed identifier with the canonical-cased completion word, so that se becomes SELECT. But the guard that skipped this for the already-typed word only matched when the case was identical (word === prefix). If you typed hash and HASH was the (invisible, selected) candidate, pressing Right silently rewrote your hash into HASH.

This broadens the guard so accepting adds nothing whenever the completion is the same word ignoring case (an empty suffix — every candidate starts with the prefix, so equal length means equal word). Real extensions such as se -> SELECT still rewrite casing as before.

Version info

  • Merged into: 26.7.1.442 (included in 26.7 and later)

In play.html, when the highlighted completion is the already-typed word in a
different case (an "empty completion", e.g. HASH offered for a typed `hash`),
pressing Right/Tab/Enter rewrote the word to the canonical case instead of
doing nothing.

Broaden the "nothing to do" guard in acceptCompletion so it fires whenever the
completion adds no characters (the same word ignoring case), while still
rewriting casing for real extensions (e.g. `se` -> `SELECT`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@clickhouse-gh

clickhouse-gh Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [d1b4001]

Summary:


AI Review

Summary

This PR updates acceptCompletion in programs/server/play.html so accepting an equal completion candidate closes the popup without rewriting the already-typed word to canonical casing, and without bumping completionUsage. I did not find any remaining correctness issues in the current diff; the earlier ranking side effect reported on the review thread is fixed in the current code.

Missing context / blind spots
  • ⚠️ There is no focused browser/UI test harness for play.html in this repository, so the keyboard and mouse completion behavior was validated by code inspection and existing CI rather than an automated regression test.
Final Verdict

✅ No new findings.

@clickhouse-gh clickhouse-gh Bot added the pr-improvement Pull request with some product improvements label Jul 2, 2026
Comment thread programs/server/play.html
Accepting the already-typed word (e.g. `HASH` offered for a typed
`hash`) is a visible no-op, but it still incremented `completionUsage`
for that word before the early return. That re-ranked later suggestions
(the scorer would prefer `HASH` the next time `ha` was typed) even
though nothing was inserted, so it was not a true no-op.

Move the usage bump below the equal-word early return so acceptance is
recorded only when we actually replace text. Real extensions such as
`se` -> `SELECT` still record usage and rewrite casing as before.

Addresses review feedback on `programs/server/play.html`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alexey-milovidov

Copy link
Copy Markdown
Member Author

Pushed d1b4001f140 addressing the AI-review "Changes required" finding: the completionUsage bump is moved below the equal-word early return in acceptCompletion, so accepting the already-typed word (hash with HASH offered) is now a genuine no-op that does not re-rank later suggestions. Details in the resolved thread above.

The only non-flake-free CI red was AST fuzzer (arm_asan_ubsan)Logical error: Not-ready Set is passed as the second argument for function (STID 0250-4e52), which is the known, tracked flake #102803 (labelled fuzz) and is unrelated to an HTML-only change. Ready for re-review once CI re-runs.

@alexey-milovidov
alexey-milovidov requested a review from pamarcos July 2, 2026 07:01
@pamarcos pamarcos self-assigned this Jul 2, 2026

@pamarcos pamarcos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and it works

@alexey-milovidov
alexey-milovidov added this pull request to the merge queue Jul 2, 2026
Merged via the queue into master with commit d78c09a Jul 2, 2026
176 checks passed
@alexey-milovidov
alexey-milovidov deleted the play-suggestion-noop branch July 2, 2026 16:40
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants