fix(dictation): stop deriving fallback target IDs from mutable placeholder text#508
Merged
Conversation
…older text UniversalDictationModule rewrites a field's placeholder attribute during recording (e.g. to "Recording..."), but getTargetElementId()'s fallback key for id-less elements baked the placeholder text in. That desyncs the key between when a target is registered and when contextual refinement looks it up later, so refinement silently no-ops with a "No element found for pending refinement target" warning on any id-less field — reproduced live on x.com/i/grok's composer while scoping baseline Grok dictation support. Cache a stable per-element fallback ID in a WeakMap instead, so it survives attribute mutations for the life of the element. Fixes #507 Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01T4uuEsKVySbRV9XRQa2ebc
Per review feedback on #508: document that the fix assumes an element's id-presence doesn't change mid-session, so a future refactor doesn't silently reintroduce the same class of key-drift bug. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01T4uuEsKVySbRV9XRQa2ebc
Contributor
Author
|
Addressed the two review suggestions:
|
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.
Summary
getTargetElementId()'s fallback identifier for id-less dictation targets baked in the element'splaceholderattribute — butUniversalDictationModuleitself rewrites that placeholder mid-session (e.g. to "Recording...") to show dictation progress, so the key used to register a target's audio/refinement state drifts from the key later used to look it up.id— reproduced live on x.com/i/grok's "Ask anything" composer while scoping baseline universal-dictation support for Grok. The local test fixture never caught this because all its fields have stableids.WeakMap, generated once and independent of any later attribute mutation.Test plan
DictationMachine-Refinement.spec.ts, "Target ID Stability (issue Dictation contextual refinement silently fails on any id-less field (target key includes mutable placeholder text) #507)") that mutates an id-less field's placeholder between segment capture and refinement; confirmed it reproduces the exact"No element found for pending refinement target"warning before the fix, and passes after.npm testgreen:tsc --noEmit, Jest (2/2), Vitest (205 files / 1795 tests, 1 pre-existing skip).Fixes #507
🤖 Generated with Claude Code
https://claude.ai/code/session_01T4uuEsKVySbRV9XRQa2ebc