Pausing mid-edit no longer discards the Cluster Strength - #376
Open
lstein wants to merge 1 commit into
Open
Conversation
An empty Cluster Strength field is a deliberate signal — it means "go back to a derived strength". The trap is that `<input type="number">` reports an empty value for anything it cannot parse *yet*: "0.", "-", "1e". So an empty field alone cannot be read as the user asking for a derived value, and pausing for a second while retyping looked exactly like it, throwing away the number the album was tuned to. `validity.badInput` separates the two: the browser sets it only while the input holds text it could not turn into a number. `Number.isNaN` cannot do this job — the sanitized value is "", never "NaN" — so the guard that was there was dead code for this element. Two smaller things in the same handler: * The pending save is now dropped before *every* early return rather than after them. Otherwise a save armed by an earlier keystroke still fires a second later, carrying a number the field no longer shows. * A non-positive number is not saved at all. DBSCAN refuses one, so `resolve_cluster_eps` floors it and the map ends up clustered at something other than the number the spinner and the cluster-info modal report. Note on the tests: jsdom sanitizes an unparseable value to "" but never sets `badInput`, so the flag is stubbed. These tests pin this module's logic; that a browser really sets `badInput` for a half-typed number is a platform guarantee, not something the suite proves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lstein
force-pushed
the
lstein/fix/mid-typing-cluster-strength
branch
from
August 20, 2026 00:18
0afca9a to
2d779f2
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
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.
Rebased onto master now that #370 has landed, and re-scoped: #370 changed what an empty field means, which changes what this bug is.
The bug
An empty Cluster Strength field is now a deliberate signal — it means "go back to a derived strength". The trap is that
<input type="number">reports an empty value for anything it cannot parse yet:So an empty field alone cannot be read as the user asking for a derived value. Pausing for a second while retyping looks exactly like it, and throws away the number the album was tuned to:
The existing guard cannot catch this.
if (eps !== null && Number.isNaN(eps))is dead code for a number input: the sanitized value is"", soreadSpinnerEpsreturnsnull, neverNaN.The fix
validity.badInputis the discriminator. The browser sets it only while the input holds text it could not turn into a number, so it separates "the user cleared this" from "the browser cannot parse this yet" — which reading.valuealone cannot do.Two smaller things in the same handler:
resolve_cluster_epsfloors it and the map ends up clustered at something other than what the spinner and the cluster-info modal report. (Refuse a Cluster Strength that DBSCAN cannot run with #375 refuses the same values server-side; this stops the UI producing them.)Tests
5 tests in
tests/frontend/umap-eps-debounce.test.js. Against master, three fail and two pass:The two that already pass are there as guards: the deliberate-clear path is the feature #370 added, and it must keep working.
Honest caveat on the main one. jsdom sanitizes an unparseable value to
""but never setsbadInput(verified — see thetypeUnparseablehelper, which stubs it). These tests pin this module's logic; that a browser really setsbadInputfor a half-typed number is a platform guarantee, not something this suite proves. Worth a manual check in a real browser before merging.597 frontend tests pass; eslint and prettier clean.
🤖 Generated with Claude Code