diff --git a/app/src/main/java/helium314/keyboard/latin/WordComposer.java b/app/src/main/java/helium314/keyboard/latin/WordComposer.java index e8d93e519..3c52171dc 100644 --- a/app/src/main/java/helium314/keyboard/latin/WordComposer.java +++ b/app/src/main/java/helium314/keyboard/latin/WordComposer.java @@ -13,6 +13,7 @@ import helium314.keyboard.keyboard.internal.keyboard_parser.floris.KeyCode; import helium314.keyboard.latin.SuggestedWords.SuggestedWordInfo; import helium314.keyboard.latin.common.ComposedData; +import helium314.keyboard.latin.common.Constants; import helium314.keyboard.latin.common.CoordinateUtils; import helium314.keyboard.latin.common.InputPointers; import helium314.keyboard.latin.common.StringUtils; @@ -68,6 +69,27 @@ public final class WordComposer { // current gesture. Pretends the user briefly paused at the prefix endpoint before // continuing the stroke — within the recogniser's "single stroke" tolerance. private static final int EXTEND_BASE_GAP_BEFORE_NEW_MS = 60; + // Prefix densification: a stored base is one key-center vertex per prefix letter, so a + // 2-letter tapped prefix is just 2 points glued to a 20+ point swipe — the recogniser scores + // by sampled-point density along the path, so the sparse prefix is out-voted by the dense + // swipe tail and words that drop the typed prefix can out-rank the intended word (tap "s","h" + // + swipe -> "would" beats "should"). We resample the prefix polyline at ~the swipe's own + // point spacing so it carries comparable weight. Fallback spacing (px) when the swipe is too + // short to measure, and a hard cap so a long prefix can never swamp the swipe. + private static final float DENSIFY_FALLBACK_SPACING_PX = 30f; + private static final int DENSIFY_MAX_BASE_POINTS = 64; + // A base whose average inter-point distance exceeds this multiple of the swipe's spacing is + // treated as the SPARSE key-center representation (a tapped prefix or a realigned-after-edit + // seed) and gets per-key clustering; anything denser is a real captured stroke (a prior swipe + // we're extending) and is passed through unchanged. + private static final float DENSIFY_SPARSE_RATIO = 2.5f; + // Cluster size (samples added per sparse key center) scales mildly with swipe richness within + // these bounds, so a tapped letter carries weight comparable to — not exceeding — a swiped one. + private static final int DENSIFY_MIN_CLUSTER = 2; + private static final int DENSIFY_MAX_CLUSTER = 4; + // Pixel step between samples within a key cluster. A few px so the cluster reads as a brief + // dwell / micro-move at the key without ever reaching a neighbouring key. + private static final float DENSIFY_CLUSTER_STEP_PX = 6f; // Cache these values for performance private CharSequence mTypedWordCache; @@ -286,18 +308,10 @@ public void setBatchInputPointers(final InputPointers batchPointers) { && batchPointers.getPointerSize() > 0) { // Multi-part composition: feed the lib the merged trail (prior fragments + // current gesture) with synthesised timestamps so the base looks like a - // natural continuation of the new gesture. - final int baseSize = mExtendBatchInputBase.getPointerSize(); - final int[] baseX = mExtendBatchInputBase.getXCoordinates(); - final int[] baseY = mExtendBatchInputBase.getYCoordinates(); - final int firstNewTime = batchPointers.getTimes()[0]; - final int baseLastTime = firstNewTime - EXTEND_BASE_GAP_BEFORE_NEW_MS; - final int baseFirstTime = baseLastTime - (baseSize - 1) * EXTEND_BASE_POINT_INTERVAL_MS; - mInputPointers.reset(); - for (int i = 0; i < baseSize; i++) { - mInputPointers.addPointer(baseX[i], baseY[i], 0, - baseFirstTime + i * EXTEND_BASE_POINT_INTERVAL_MS); - } + // natural continuation of the new gesture. The base is resampled to the swipe's + // own density first (see appendDensifiedExtendBase) so the typed prefix isn't + // out-weighted by the dense swipe tail. + appendDensifiedExtendBase(batchPointers); mInputPointers.appendAll(batchPointers); } else { mInputPointers.set(batchPointers); @@ -305,6 +319,116 @@ public void setBatchInputPointers(final InputPointers batchPointers) { mIsBatchMode = true; } + /** + * Build the merged-trail prefix into {@link #mInputPointers}, adding weight to a SPARSE + * key-center base so the recogniser respects the typed prefix. + *
+ * The stored base ({@link #mExtendBatchInputBase}) is one of two things: + *
+ * Phase 1 of {@code docs/COMPOSING_WORD_SOURCE_OF_TRUTH.md}: after a fragment-pop / + * partial delete truncates the composing word, the stored stroke still holds the longer + * pre-edit geometry (reset() does not clear it, and addPointerAt overwrites in place + * without shrinking the length). A following swipe-extend would then snapshot that stale + * buffer as its merged-trail base and build an ever-longer word. Rebuilding the buffer + * from the truncated word's key centers keeps the stroke aligned with the text. This is a + * local preview of the broader "derive stroke from text" direction. + * @param codePoints the code points of the (truncated) word + * @param coordinates key-center x/y in CoordinateUtils format (e.g. from + * {@code getCoordinatesForCurrentKeyboard}) + */ + public void seedInputPointersFromKeyCenters(final int[] codePoints, final int[] coordinates) { + mInputPointers.reset(); + final int count = (codePoints == null) ? 0 : codePoints.length; + for (int i = 0; i < count; i++) { + final int x = CoordinateUtils.xFromArray(coordinates, i); + final int y = CoordinateUtils.yFromArray(coordinates, i); + // A code point the current layout can't resolve (symbol, or a case mismatch in the + // lookup) has no key geometry. NOT_A_COORDINATE fed to the gesture recognizer as a + // real point would warp the whole stroke toward (-1,-1), so skip it — a slightly + // shorter seed is far better than a corrupted one. + if (x == Constants.NOT_A_COORDINATE || y == Constants.NOT_A_COORDINATE) continue; + mInputPointers.addPointer(x, y, 0, 0); + } + } + /** * Set the currently composing word to the one passed as an argument. * This will register NOT_A_COORDINATE for X and Ys, and use the passed keyboard for proximity. @@ -431,6 +589,19 @@ public void setCapitalizedModeAtStartComposingTime(final int mode) { mCapitalizedMode = mode; } + /** + * The capitalization intent captured when this word started composing (one of the + * {@code CAPS_MODE_*} constants). It is seeded from auto-cap + shift state at word-start, + * survives {@link #reset()} (so it persists across a {@link #setBatchInputWord} rebuild), and + * is cleared only in {@link #commitWord}. This makes it the persistent per-word source of + * truth for casing — used by the live-converge re-recognition path so that re-replacing the + * word never loses (or latches) its case. + * @return the capitalized mode for the current word + */ + public int getCapitalizedMode() { + return mCapitalizedMode; + } + /** * Before fetching suggestions, we don't necessarily know about the capitalized mode yet. *
diff --git a/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java b/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java index 834cc1561..f05009402 100644 --- a/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java +++ b/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java @@ -120,10 +120,11 @@ public final class InputLogic { // GESTURE-START, not gesture-end (so a long gesture doesn't lose the promotion). private boolean mGestureExtendsByTapPromotion; - // Snapshot of {@code keyboardSwitcher.getKeyboardShiftMode()} captured at the start of - // each gesture. Used by {@link #onUpdateTailBatchInputCompleted} to capitalize the - // recognizer's lowercase output. We can't read getKeyboardShiftMode() at gesture-end - // because the keyboard typically auto-clears the shifted state during the gesture. + // Snapshot of {@code keyboardSwitcher.getKeyboardShiftMode()} captured at the very start of + // each gesture, BEFORE any state mutates (a prior word may auto-commit / the shift indicator + // may auto-clear within onStartBatchInput). Used ONLY by the FRESH-word capitalization in + // {@link #onUpdateTailBatchInputCompleted}. The live-converge / multi-part EXTEND path uses the + // persistent {@link WordComposer#getCapitalizedMode()} instead — see that method's javadoc. private int mShiftModeAtGestureStart = WordComposer.CAPS_MODE_OFF; /** Set to true at the end of {@link #onCombiningGraceExpired} when an autospace was * written, so the next punctuation tap in {@link #handleSeparatorEvent} can strip it @@ -157,6 +158,18 @@ public final class InputLogic { private boolean mInCombiningMode; private boolean mCombiningWordHasGestureFragment; + // True when the composing word has been EDITED (a delete of any kind) since the raw stroke + // buffer (WordComposer#mInputPointers) was last built to match it. A delete shrinks the typed + // word but never shrinks the buffer, so the buffer goes stale — and a following swipe-extend + // that arms its base from the buffer rebuilds a longer word (delete "ng" from "Thing" -> "Thi", + // swipe "ng" -> "Whining"). At the one place the stroke is consumed as a swipe-extend base + // (onStartBatchInput), if this is set we realign the base from the authoritative text instead. + // Set on every backspace (handleBackspaceEvent); cleared whenever the buffer is rebuilt to + // match the word — a gesture (onUpdateTailBatchInputCompleted) — or the word ends (commit, + // reset). NOT set for a plain tap-then-gesture (no delete), so that continuous swipe+swipe and + // tap+swipe keep their real captured stroke. + private boolean mComposingStrokeStale; + // Live-converge (multi-part, opt-in PREF_MULTIPART_RERECOGNIZE_TAPS): the accumulated raw // pointer trail of the CURRENT word — the latest gesture fragment's points plus any tap // key-centers appended by {@link #tryLiveConvergeTap}. This must persist across fragments @@ -731,10 +744,11 @@ public void onStartBatchInput(final SettingsValues settingsValues, markForceNextSpaceWordStarted(); // Snapshot the keyboard's shift mode BEFORE any state mutates — the shifted indicator // typically auto-clears once the gesture starts moving, so by the time - // onUpdateTailBatchInputCompleted fires the live mode reads as UNSHIFTED. - // We compute the *actual* caps mode (resolves AUTO_SHIFTED into AUTO_SHIFT_LOCKED if - // the input field is in all-caps), so a true all-caps field gives the right answer. - mShiftModeAtGestureStart = getActualCapsMode(settingsValues, keyboardSwitcher.getKeyboardShiftMode()); + // onUpdateTailBatchInputCompleted fires the live mode reads as UNSHIFTED. This drives the + // FRESH-word capitalization only; the EXTEND path uses WordComposer.mCapitalizedMode (the + // persistent per-word intent). We compute the *actual* caps mode (resolves AUTO_SHIFTED + // into AUTO_SHIFT_LOCKED if the input field is all-caps) so a true all-caps field is right. + mShiftModeAtGestureStart = gestureStartCapsMode(settingsValues, keyboardSwitcher.getKeyboardShiftMode()); mWordBeingCorrectedByCursor = null; mInputLogicHandler.onStartBatchInput(); handler.showGesturePreviewAndSetSuggestions(SuggestedWords.getEmptyBatchInstance(), false); @@ -802,7 +816,26 @@ public void onStartBatchInput(final SettingsValues settingsValues, // word simply stays open until the user taps space), see // SettingsValues#isMultipartComposeActive. if (settingsValues.isMultipartComposeActive()) { + // The extend base is the geometry every setBatchInputPointers call prepends. + // mInputPointers is only trustworthy if the composing text still matches what + // the last gesture produced (continuous swipe+swipe). If the text was edited + // since — single-char backspace, fragment-pop, selection / multi-char delete, + // cursor re-compose — the buffer is stale (it never shrinks on a delete), so + // rebuild the base from the authoritative text. This is the single consumption + // point, so realigning here fixes the whole class regardless of HOW the text + // was edited (delete "ng" from "Thing" -> "Thi", swipe "ng" -> "Thing", not + // "Whining"). Equal text -> keep the real captured stroke (higher fidelity). + final String composingNow = mWordComposer.getTypedWord(); + if (mComposingStrokeStale) { + realignComposerStrokeToText(composingNow, settingsValues); + } mWordComposer.setExtendBatchInputBase(mWordComposer.getInputPointers()); + if (settingsValues.mGestureDebugDrawPoints) { + Log.d(TAG, "extend arm baseSize=" + + mWordComposer.getExtendBatchInputBaseSize() + + " composing='" + composingNow + "'" + + " realigned=" + mComposingStrokeStale); + } } } else if (mWordComposer.isSingleLetter() && !isInlineEmojiSearchAction()) { // We auto-correct the previous (typed, not gestured) string iff it's one @@ -853,8 +886,27 @@ public void onStartBatchInput(final SettingsValues settingsValues, } } mConnection.endBatchEdit(); - mWordComposer.setCapitalizedModeAtStartComposingTime( - getActualCapsMode(settingsValues, keyboardSwitcher.getKeyboardShiftMode())); + // Capture the word's casing intent ONLY when this gesture starts a fresh word. When it + // EXTENDS an existing composing word (multi-part swipe+swipe / manual spacing), the intent + // belongs to the word's first fragment and must be preserved: the keyboard auto-clears its + // shifted indicator after the first gesture, so re-capturing here would read UNSHIFTED and + // wrongly downcase a word that started capitalized ("Was"+swipe -> "wait" instead of + // "Wait"). mCapitalizedMode survives the setBatchInputWord rebuild and is cleared at + // commitWord, so leaving it untouched keeps the original intent alive across the extension. + // (Live-converge tap extensions bypass onStartBatchInput entirely, so they were already + // safe; this closes the same gap for swipe extensions.) + if (!extendComposingWord) { + mWordComposer.setCapitalizedModeAtStartComposingTime( + gestureStartCapsMode(settingsValues, keyboardSwitcher.getKeyboardShiftMode())); + } + if (settingsValues.mGestureDebugDrawPoints) { + Log.d(TAG, "caps@start extend=" + extendComposingWord + + " recapture=" + (!extendComposingWord) + + " capsMode=" + mWordComposer.getCapitalizedMode() + + " composing='" + mWordComposer.getTypedWord() + "'" + + " cursorFrontOrMid=" + mWordComposer.isCursorFrontOrMiddleOfComposingWord() + + " stale=" + mComposingStrokeStale); + } } /* @@ -1331,6 +1383,37 @@ private void onCombiningGraceExpired() { getCurrentAutoCapsState(sv), getCurrentRecapitalizeState()); } + /** + * Realign the composer's raw stroke buffer ({@code WordComposer#mInputPointers}) to + * {@code word}'s key centers. + *
+ * Phase 1 of {@code docs/COMPOSING_WORD_SOURCE_OF_TRUTH.md}, generalized: the stroke buffer + * is parallel state that drifts stale whenever the composing word is edited by anything other + * than a gesture — a fragment-pop, a single-char backspace, a selection / multi-char delete, + * or a cursor-driven re-compose. None of those shrink {@code mInputPointers}, so a following + * swipe-extend that arms its base from the buffer rebuilds a longer word. Rebuilding from the + * surviving text's key centers keeps the base aligned. Lowercases for the exact key lookup + * ({@link Keyboard#getCoordinates} matches lowercase layout keys); the seed skips unresolvable + * keys so one bad point can't warp the stroke toward (-1,-1). + */ + private void realignComposerStrokeToText(final String word, final SettingsValues sv) { + final int[] cps = StringUtils.toCodePointArray(word.toLowerCase(sv.mLocale)); + final int[] coords = mLatinIME.getCoordinatesForCurrentKeyboard(cps); + // Defensive: if the layout resolves NO key, the seed would empty the buffer and disarm the + // extend entirely, so we leave the buffer untouched. This covers: no keyboard yet, an + // all-symbol word, AND a letters-word while a non-letter layer (symbols/numeric) is active — + // in the last case the leftover base is fully stale, but you can't gesture-type letters on a + // symbol layer anyway, so it's effectively unreachable; and emptying here would also break + // unit tests whose harness has no resolvable layout. A slightly-stale base beats no base. + // (coords is CoordinateUtils format: x at even indices, y at odd.) + boolean anyResolved = false; + for (int i = 0; i < coords.length; i += 2) { + if (coords[i] != Constants.NOT_A_COORDINATE) { anyResolved = true; break; } + } + if (!anyResolved) return; + mWordComposer.seedInputPointersFromKeyCenters(cps, coords); + } + /** * Try to handle a backspace as a fragment-pop rather than a char-delete. Returns true if * handled (the caller should then return without touching the editor further); false if @@ -1376,6 +1459,18 @@ private boolean tryFragmentBackspace(final SettingsValues sv) { // appends correctly and the suggestion strip looks at it as typed text. mWordComposer.setBatchInputWord(newWord); mWordComposer.unsetBatchMode(); + // Phase 1 (COMPOSING_WORD_SOURCE_OF_TRUTH.md): realign the raw stroke buffer to the + // truncated word's key centers. setBatchInputWord leaves mInputPointers at the + // longer pre-pop geometry; without this, a following swipe-extend would snapshot + // that stale buffer as its merged-trail base and build an ever-longer garbage word. + // (See realignComposerStrokeToText.) The swipe-extend arm site also realigns when the + // text was edited, so this is belt-and-suspenders, but doing it here keeps the buffer + // consistent for any intervening consumer too. + realignComposerStrokeToText(newWord, sv); + if (sv.mGestureDebugDrawPoints) { + Log.d(TAG, "fragment pop '" + oldWord + "' -> '" + newWord + + "' seededPointers=" + mWordComposer.getInputPointers().getPointerSize()); + } setComposingTextInternal(newWord, 1); } mConnection.endBatchEdit(); @@ -1404,6 +1499,8 @@ public void setSuggestedWords(final SuggestedWords suggestedWords) { mWordComposer.setAutoCorrection(suggestedWordInfo); } mSuggestedWords = suggestedWords; + // #24 spacing-policy signals (upstream): independent per-keystroke signals derived from the + // same suggestion results; passive groundwork, consumed by the upcoming signal-driven grace. final SpacingSignals spacingSignals = computeSpacingSignals(suggestedWords); mSpacingComplete = spacingSignals.complete; mSpacingPrefixRichScore = spacingSignals.prefixRichScore; @@ -2436,6 +2533,12 @@ private void handleBackspaceEvent(final Event event, final InputTransaction inpu // explicitly retracting input; we don't want the timer to fire mid-correction. cancelCombiningMode(); clearOneShotSpaceActionAndNotifyIfChanged(); + // A backspace of ANY kind (single-char, fragment-pop, whole-word, selection / multi-char + // delete) shrinks the typed word but never shrinks the raw stroke buffer, so the buffer is + // now stale relative to the text. Mark it: the next swipe-extend will realign its base from + // the text instead of arming the leftover (longer) stroke. Set once here, before any + // backspace branch runs, so every delete path is covered from the single consumption point. + mComposingStrokeStale = true; // Live-converge (#1.7): a backspace invalidates the accumulated gesture stroke — we // can't reliably trim the raw trail to match a partially-deleted word, and leaving it // intact would make the NEXT swipe/tap merge with stale geometry (e.g. delete "ing" @@ -2560,6 +2663,14 @@ private void handleBackspaceEvent(final Event event, final InputTransaction inpu // composing span in the editor. commitText("", 1) is the correct primitive for // removing composing text — unlike deleteTextBeforeCursor. mConnection.commitText("", 1); + // The composing word's fragment boundaries index into a word that no longer + // exists. The batch- and whole-word-delete branches above reach here via + // mWordComposer.reset() (which doesn't touch the boundary stack), and a + // char-delete that removed the last letter reaches here too. Drop the stale + // boundaries at this single exit so they can't leak into the next word's + // fragment math (a leaked boundary <= the next word's length would otherwise + // make a later fragment-backspace shrink to the wrong length). + clearFragmentBoundaries(); } updateInlineEmojiSearch(); inputTransaction.setRequiresUpdateSuggestions(); @@ -3445,6 +3556,23 @@ private int getActualCapsMode(final SettingsValues settingsValues, return WordComposer.CAPS_MODE_OFF; } + /** + * Caps mode to seed a STARTING gesture's casing. {@link #getActualCapsMode} only resolves + * auto-caps when the visual shift mode is already AUTO_SHIFTED, but that indicator updates + * asynchronously — a swipe that begins right at a sentence/field start can still read UNSHIFTED, + * dropping the leading capital (swiping the first word of a sentence gave "yeah" instead of + * "Yeah"). When the visual mode is OFF but auto-caps genuinely applies at the cursor (computed + * from context and gated by the user's auto-capitalization setting), treat it as AUTO_SHIFTED. + */ + private int gestureStartCapsMode(final SettingsValues settingsValues, final int keyboardShiftMode) { + int shiftMode = keyboardShiftMode; + if (shiftMode == WordComposer.CAPS_MODE_OFF + && getCurrentAutoCapsState(settingsValues) != Constants.TextUtils.CAP_MODE_OFF) { + shiftMode = WordComposer.CAPS_MODE_AUTO_SHIFTED; + } + return getActualCapsMode(settingsValues, shiftMode); + } + /** * Gets the current auto-caps state, factoring in the space state. *
@@ -3634,6 +3762,14 @@ private void resetComposingState(final boolean alsoResetLastComposedWord) { // Two-thumb typing (#1.1): composing word is wiped — drop the matching boundaries. clearFragmentBoundaries(); mCombiningWordHasGestureFragment = false; + // The composing word is wiped, but reset() deliberately does NOT shrink mInputPointers + // (touch coords are kept for cursor-move resume), so the raw stroke buffer is now stale + // relative to whatever word comes next. Mark it stale: a following swipe-extend must + // realign its base from the new text rather than arm the leftover geometry. This is the + // path the backspace handler doesn't cover — e.g. select-and-delete then retype, or a + // cursor-move reset — where the next gesture would otherwise extend the deleted word's + // stroke ("Should" deleted, "W" tapped, swipe -> "Whirlpools"). + mComposingStrokeStale = true; // Live-converge (#1.7): the word is gone, so its accumulated stroke is stale. mLiveStroke.reset(); // Two-thumb typing: likewise drop the merged-trail extend-base. This path also covers @@ -3813,6 +3949,44 @@ private static boolean isPlainLetterWord(final String s) { return true; } + /** + * Apply a word's casing INTENT to a casing-neutral lemma. Used by the live-converge + * (merged-trail) re-recognition path, which replaces the whole composing word on every + * extending tap. The recognizer's output is treated as letters only — its own casing (e.g. an + * all-caps dictionary acronym that the engine happened to rank first) is discarded by + * lowercasing — and the case is then re-derived from the persistent per-word intent + * ({@link WordComposer#getCapitalizedMode()}, seeded at word-start from auto-cap + shift and + * alive until commit). This keeps a sentence-start capital across every re-converge (issue #5) + * and, because the lemma is lowercased first, prevents an unsolicited all-caps result from + * latching the whole word in caps (issue #4). + * + *
Pure function of its inputs (no engine / native lib), so the casing behaviour is unit + * testable directly. {@code public static} for that reason. + * + * @param lemma the recognizer's word output (casing not trusted) + * @param capitalizedMode one of the {@link WordComposer} {@code CAPS_MODE_*} constants + * @param locale locale for case mapping + * @return the lemma cased to match the intent + */ + public static String applyComposingCase(final String lemma, final int capitalizedMode, + final Locale locale) { + if (lemma == null || lemma.isEmpty()) return lemma; + final String lower = lemma.toLowerCase(locale); + switch (capitalizedMode) { + case WordComposer.CAPS_MODE_AUTO_SHIFT_LOCKED: + case WordComposer.CAPS_MODE_MANUAL_SHIFT_LOCKED: + // Deliberate caps-lock (or an all-caps input field) — uppercase the whole word. + return lower.toUpperCase(locale); + case WordComposer.CAPS_MODE_AUTO_SHIFTED: + case WordComposer.CAPS_MODE_MANUAL_SHIFTED: + // Sentence-start / shift — first letter only. + return StringUtils.capitalizeFirstCodePoint(lower, locale); + default: + // CAPS_MODE_OFF — no intent, leave the neutral lemma lowercase. + return lower; + } + } + private boolean textBeforeCursorMayBeUrlOrSimilar(final SettingsValues settingsValues, final Boolean forAutoSpace) { // URL / mail field and no space -> may be URL if (InputTypeUtils.isUriOrEmailType(settingsValues.mInputAttributes.mInputType) && @@ -3864,6 +4038,8 @@ public void onUpdateTailBatchInputCompleted(final SettingsValues settingsValues, + " top=" + batchInputText + " composingBefore=" + mWordComposer.getTypedWord() + " extendBase=" + mWordComposer.isExtendBatchInputBaseSet() + + " baseSize=" + mWordComposer.getExtendBatchInputBaseSize() + + " mergedPts=" + mWordComposer.getInputPointers().getPointerSize() + " candidates=[" + candidates + "]"); } // Multi-part word composition (#1.6): when the merged-trail extend-base path was @@ -3882,6 +4058,11 @@ public void onUpdateTailBatchInputCompleted(final SettingsValues settingsValues, } } if (TextUtils.isEmpty(batchInputText)) { + // The gesture consumed/merged the stroke buffer but recognized nothing. On an EXTEND + // the buffer now holds the merged (prior + this) trail while the composing text is + // unchanged, so it's stale — mark it so the next swipe-extend realigns from the text + // instead of arming this leftover (doubled) geometry. + mComposingStrokeStale = true; // Still need to clear the seed slot so it doesn't leak into the next gesture. helium314.keyboard.keyboard.PointerTracker.consumeGestureSeedCodepoint(); return; @@ -3907,7 +4088,12 @@ public void onUpdateTailBatchInputCompleted(final SettingsValues settingsValues, batchInputText = batchInputText.substring(Character.charCount(firstCp)); } } - if (batchInputText.isEmpty()) return; + if (batchInputText.isEmpty()) { + // Seed-strip left nothing (the recognized word was only the seeded letter): same stale + // situation as above — the merged buffer no longer matches the composing text. + mComposingStrokeStale = true; + return; + } mCombiningWordHasGestureFragment = true; mConnection.beginBatchEdit(); // Two-thumb typing (#1.1 + #1.4): when either manual spacing OR tap-promotion-extend @@ -3962,9 +4148,12 @@ public void onUpdateTailBatchInputCompleted(final SettingsValues settingsValues, // those continuation gestures should append in the casing the user already chose for // the start of the word. if (!extendExistingCompose && !batchInputText.isEmpty()) { - // Use the shift mode captured at gesture-start, not the live mode — the - // keyboard auto-clears the shifted indicator during the gesture, so a live - // read here always returns UNSHIFTED. + // Fresh-word capitalization: use the shift mode captured at gesture-start, not the live + // mode — the keyboard auto-clears the shifted indicator during the gesture, so a live + // read here always returns UNSHIFTED. This is the long-standing path for a plain single + // gesture; it ADDS a capital to the recognizer's lowercase output and deliberately does + // NOT neutralize an intrinsic all-caps result, so a standalone acronym swipe ("CSA") + // stays as-is. (The EXTEND path below handles re-cased re-recognition separately.) final int shiftMode = mShiftModeAtGestureStart; if (shiftMode == WordComposer.CAPS_MODE_MANUAL_SHIFTED || shiftMode == WordComposer.CAPS_MODE_AUTO_SHIFTED) { @@ -3974,8 +4163,60 @@ public void onUpdateTailBatchInputCompleted(final SettingsValues settingsValues, batchInputText = batchInputText.toUpperCase(settingsValues.mLocale); } } - // Clear so a stale value from a previous gesture can't leak into a non-gesture - // commit later. + // Live-converge (#1.7) casing — gated to the merged-trail re-recognition path, so a plain + // single gesture (incl. a standalone acronym swipe, handled by the block above) is + // untouched. A merged-trail commit REPLACES the whole word with the recognizer's fresh + // output on every extending tap, which otherwise: + // - dropped the first-letter capital the word started with (issue #5); and + // - could latch an unsolicited all-caps acronym ("CSA"), which then stuck the whole word + // in caps via WordComposer.isAllUpperCase forcing every later suggestion upper (#4). + // The fix treats the recognizer output as a casing-NEUTRAL lemma and re-applies the word's + // persistent intent (WordComposer.mCapitalizedMode). Lowercasing the lemma first is what + // dissolves #4 at the source: the composing word is never all-caps, so isAllUpperCase + // never arms and Suggest stops force-uppercasing — no shift-lock special-case needed. + if (usedMergedTrail) { + final String preCaseLemma = batchInputText; + // mCapitalizedMode is the persistent per-word caps intent, but commitWord() zeroes it. + // After a word is auto-committed (e.g. an autospace) and then re-composed by backspacing + // back into it, the intent is lost (CAPS_MODE_OFF) even though the visible composing text + // is still capitalized ("Thi"). Because the merged-trail recognizer REPLACES the whole + // word with a lowercase lemma, relying on capsMode alone downcases a word the user sees + // as capitalized. Recover the intent from the existing composing word's own case when + // capsMode has lost it: all-caps -> shift-locked (keeps "CSA"), else first-letter shift. + int effectiveCapsMode = mWordComposer.getCapitalizedMode(); + final String existingWord = mWordComposer.getTypedWord(); + if (effectiveCapsMode == WordComposer.CAPS_MODE_OFF + && existingWord != null && !existingWord.isEmpty() + && Character.isUpperCase(existingWord.codePointAt(0))) { + // Only treat the existing word as a deliberate ALL-CAPS run (shift-lock) when it + // has at least two letters; a lone leading capital like "I" or "A" is just a + // sentence-start cap, so use first-letter shift — otherwise the whole re-recognized + // word would be upper-cased ("I" -> "ILL", "A" -> "AND"). + int letterCount = 0; + for (int ci = 0; ci < existingWord.length(); ) { + final int cp = existingWord.codePointAt(ci); + if (Character.isLetter(cp)) letterCount++; + ci += Character.charCount(cp); + } + final boolean deliberateAllCaps = + letterCount >= 2 && StringUtils.isIdenticalAfterUpcase(existingWord); + effectiveCapsMode = deliberateAllCaps + ? WordComposer.CAPS_MODE_MANUAL_SHIFT_LOCKED + : WordComposer.CAPS_MODE_MANUAL_SHIFTED; + } + batchInputText = applyComposingCase(batchInputText, effectiveCapsMode, + settingsValues.mLocale); + if (settingsValues.mGestureDebugDrawPoints) { + Log.d(TAG, "caps@extend capsMode=" + mWordComposer.getCapitalizedMode() + + " effective=" + effectiveCapsMode + + " existing='" + existingWord + "'" + + " prevTyped='" + prevTypedWord + "'" + + " lemma='" + preCaseLemma + "' -> '" + batchInputText + "'"); + } + } + // Clear the fresh-word snapshot so a stale value from this gesture can't leak into a + // later non-gesture commit. (The persistent extend intent lives in mCapitalizedMode and + // is cleared by WordComposer.commitWord, not here.) mShiftModeAtGestureStart = WordComposer.CAPS_MODE_OFF; final String composedText = prevTypedWord + batchInputText; // Trace recorder (A3a): capture gesture trace + committed word for replay debugging. @@ -4012,6 +4253,9 @@ public void onUpdateTailBatchInputCompleted(final SettingsValues settingsValues, mLiveStroke.set(mWordComposer.getInputPointers()); } mWordComposer.setBatchInputWord(composedText); + // The raw stroke buffer now matches this word (the gesture just (re)built it), so a + // following swipe-extend can trust it — until an edit marks it stale again. + mComposingStrokeStale = false; setComposingTextInternal(composedText, 1); if (extendExistingCompose) { // Two-thumb typing (#1.1 + #1.4): downgrade the composer out of batch mode so @@ -4025,7 +4269,17 @@ public void onUpdateTailBatchInputCompleted(final SettingsValues settingsValues, // the gesture and reflects only the LAST fragment, which would mislead the user // if they tapped it (replacing the whole composing span with one fragment). Two // options: blank the strip (legacy) or repopulate it for the full composing word. - if (settingsValues.mMultipartFullWordSuggestions) { + if (usedMergedTrail) { + // Merged-trail extend: the recognizer saw the WHOLE merged stroke, so its + // candidates are whole words ([speer, super, supper, ...]) — not fragments. Keep + // them on the strip so the recognizer's own alternatives stay one tap away when it + // ranks the wrong one first (e.g. "super" when "speer" won). Picking one routes + // through commitChosenWord (which reads mSuggestedWords) and cleanly replaces the + // composing word — there is no fragment data-loss here, which is the only reason the + // concatenation branch below re-runs typed-word suggestions instead. + setSuggestedWords(suggestedWords); + mSuggestionStripViewAccessor.setSuggestions(suggestedWords); + } else if (settingsValues.mMultipartFullWordSuggestions) { performUpdateSuggestionStripSync(settingsValues, SuggestedWords.INPUT_STYLE_TYPING); } else { setSuggestedWords(SuggestedWords.getEmptyInstance()); @@ -4277,6 +4531,8 @@ private void commitChosenWord(final SettingsValues settingsValues, final String // boundaries are now stale and would point past the end of the (empty) word. clearFragmentBoundaries(); mCombiningWordHasGestureFragment = false; + // Word committed — next word starts with a clean (not-stale) slate. + mComposingStrokeStale = false; // Live-converge (#1.7): word committed — its accumulated stroke no longer applies. mLiveStroke.reset(); // Two-thumb typing: the merged-trail extend-base belongs to the just-committed word too. diff --git a/app/src/test/java/helium314/keyboard/latin/InputLogicTest.kt b/app/src/test/java/helium314/keyboard/latin/InputLogicTest.kt index d3f2accea..11909913d 100644 --- a/app/src/test/java/helium314/keyboard/latin/InputLogicTest.kt +++ b/app/src/test/java/helium314/keyboard/latin/InputLogicTest.kt @@ -724,6 +724,91 @@ class InputLogicTest { assertFalse(composer.isExtendBatchInputBaseSet) } + // --- Live-converge casing (#4 stuck all-caps, #5 dropped auto-cap). The merged-trail + // re-recognition path replaces the whole composing word on every extending tap; its casing is + // derived from the persistent per-word intent (WordComposer.mCapitalizedMode) applied to a + // casing-NEUTRAL lemma, via InputLogic.applyComposingCase. That transform is a pure function + // (no native engine), so the casing behaviour is testable here directly — the on-device + // recognition that produces the lemma is not (no gesture lib / tap coords in the JVM harness). + private val enLocale = "en".constructLocale() + + // #5: a word that started capitalized (sentence-start / shift) keeps its leading capital when + // the recognizer re-resolves it lowercase on an extending tap. "Hello" must not become "hellow". + @Test fun applyComposingCaseKeepsLeadingCapitalAcrossReconverge() { + assertEquals("Hellow", + InputLogic.applyComposingCase("hellow", WordComposer.CAPS_MODE_AUTO_SHIFTED, enLocale)) + assertEquals("Hellow", + InputLogic.applyComposingCase("hellow", WordComposer.CAPS_MODE_MANUAL_SHIFTED, enLocale)) + } + + // #4: an unsolicited all-caps recognizer result ("CSA") is neutralized to the word's intent + // instead of latching. Sentence-start intent -> "Can"; no intent (mid-sentence) -> "can". + // This is the exact case the old prior-word heuristic got wrong (it produced "can" at a + // sentence start because it could only see the previous all-caps fragment). + @Test fun applyComposingCaseNeutralizesUnsolicitedAllCaps() { + assertEquals("Can", + InputLogic.applyComposingCase("CAN", WordComposer.CAPS_MODE_AUTO_SHIFTED, enLocale)) + assertEquals("can", + InputLogic.applyComposingCase("CAN", WordComposer.CAPS_MODE_OFF, enLocale)) + } + + // Deliberate caps-lock (or an all-caps input field) still produces all-caps on the + // merged-trail path — the one case where the whole word legitimately stays upper. + @Test fun applyComposingCaseUppercasesUnderShiftLock() { + assertEquals("CAME", + InputLogic.applyComposingCase("came", WordComposer.CAPS_MODE_MANUAL_SHIFT_LOCKED, enLocale)) + assertEquals("CAME", + InputLogic.applyComposingCase("CAME", WordComposer.CAPS_MODE_AUTO_SHIFT_LOCKED, enLocale)) + } + + // No casing intent: the neutral lemma is left lowercase regardless of the recognizer's own + // casing, so a mid-sentence re-converge never injects stray capitals. + @Test fun applyComposingCaseLeavesLowercaseWhenNoIntent() { + assertEquals("game", + InputLogic.applyComposingCase("Game", WordComposer.CAPS_MODE_OFF, enLocale)) + assertEquals("game", + InputLogic.applyComposingCase("game", WordComposer.CAPS_MODE_OFF, enLocale)) + } + + // Defensive: empty / null lemma passes through untouched (the caller also guards, but the + // helper must be safe on its own). + @Test fun applyComposingCaseHandlesEmptyAndNull() { + assertEquals("", InputLogic.applyComposingCase("", WordComposer.CAPS_MODE_AUTO_SHIFTED, enLocale)) + assertEquals(null, InputLogic.applyComposingCase(null, WordComposer.CAPS_MODE_AUTO_SHIFTED, enLocale)) + } + + // An EXTENDING gesture (swipe+swipe / manual-spacing multi-part) must NOT re-capture the + // word's casing intent at onStartBatchInput. The keyboard auto-clears its shift indicator + // after the first gesture, so re-capturing would overwrite the word-start intent with + // UNSHIFTED and downcase a capitalized word ("Was"+swipe -> "wait" instead of "Wait", the + // on-device regression this guards). Intent must survive to the merged-trail casing step. + // (Live-converge tap extensions bypass onStartBatchInput, so they were never affected.) + @Test fun extendingGestureStartPreservesCasingIntent() { + reset() + latinIME.prefs().edit { putBoolean(Settings.PREF_GESTURE_MANUAL_SPACING, true) } + chainInput("wa") // open a composing word, cursor at end + // Simulate the word having started while shifted (as "Was" did on-device). + composer.setCapitalizedModeAtStartComposingTime(WordComposer.CAPS_MODE_MANUAL_SHIFTED) + // A second gesture starts to EXTEND it (manual spacing -> extendComposingWord = true). + inputLogic.onStartBatchInput(settingsValues, KeyboardSwitcher.getInstance(), latinIME.mHandler) + handleMessages() + // Pre-fix this was clobbered to CAPS_MODE_OFF by the unconditional re-capture. + assertEquals(WordComposer.CAPS_MODE_MANUAL_SHIFTED, composer.capitalizedMode) + } + + // Sanity: a FRESH gesture (no composing word) still captures the current intent — the guard + // must not freeze a stale mode from a previous word. + @Test fun freshGestureStartStillCapturesCasingIntent() { + reset() + latinIME.prefs().edit { putBoolean(Settings.PREF_GESTURE_MANUAL_SPACING, true) } + composer.setCapitalizedModeAtStartComposingTime(WordComposer.CAPS_MODE_MANUAL_SHIFT_LOCKED) + // No composing word -> extendComposingWord = false -> intent is re-captured from the + // keyboard (not shift-locked on a fresh field), not frozen at the stale locked value. + inputLogic.onStartBatchInput(settingsValues, KeyboardSwitcher.getInstance(), latinIME.mHandler) + handleMessages() + assertFalse(composer.capitalizedMode == WordComposer.CAPS_MODE_MANUAL_SHIFT_LOCKED) + } + // Static-seed reachability guard. PointerTracker's tap-seed path (sLastLetterTap*) is gated // on (!isMultipartComposeActive() && mCombiningGraceMs > 0). But grace > 0 forces multi-part // composition active, so that conjunction is unsatisfiable and the seed is currently diff --git a/app/src/test/java/helium314/keyboard/latin/WordComposerTest.java b/app/src/test/java/helium314/keyboard/latin/WordComposerTest.java index e9c867895..35f1daa87 100644 --- a/app/src/test/java/helium314/keyboard/latin/WordComposerTest.java +++ b/app/src/test/java/helium314/keyboard/latin/WordComposerTest.java @@ -102,13 +102,16 @@ public void testExtendBatchInputBaseMergesAndRetimes() { wordComposer.setBatchInputPointers(batch); final InputPointers merged = wordComposer.getInputPointers(); - // base (2) + new gesture (3) = 5, fed to the recognizer as one continuous stroke. + // This base's points are close together (~swipe density), so it is treated as an already + // DENSE real stroke and passed through verbatim (only sparse key-center bases get the + // per-key clustering — see testFragmentPopThenReswipeUsesSeededBaseNotStaleTrail). So the + // merged stroke is base(2) + new gesture(3) = 5, fed as one continuous stroke. assertEquals(5, merged.getPointerSize()); final int[] xs = merged.getXCoordinates(); final int[] times = merged.getTimes(); - // Base coordinates come first, in order, untouched. + // Dense base coordinates come first, in order, untouched. assertEquals(10, xs[0]); assertEquals(30, xs[1]); // New gesture coordinates follow, untouched. @@ -130,6 +133,164 @@ public void testExtendBatchInputBaseMergesAndRetimes() { assertTrue("base must end before the new gesture begins", times[1] < times[2]); } + // Phase 1 (COMPOSING_WORD_SOURCE_OF_TRUTH.md): after a fragment-pop truncates the word, the + // raw stroke buffer still holds the longer pre-pop geometry (reset() doesn't clear it; the + // length doesn't shrink). seedInputPointersFromKeyCenters realigns it to the truncated word's + // key centers, so a following swipe-extend merges with geometry that matches the text instead + // of building an ever-longer garbage word. + @Test + public void testSeedInputPointersFromKeyCentersRealignsToText() { + final WordComposer wordComposer = new WordComposer(); + + // Stale 5-point stroke left by a prior, longer word. + final InputPointers stale = new InputPointers(16); + for (int i = 0; i < 5; i++) { + stale.addPointer(i, i, 0, i * 10); + } + wordComposer.setBatchInputPointers(stale); + assertEquals(5, wordComposer.getInputPointers().getPointerSize()); + + // reset() deliberately does NOT clear mInputPointers — the stale buffer survives. + wordComposer.reset(); + assertEquals(5, wordComposer.getInputPointers().getPointerSize()); + + // Realign to a 2-char truncated word's key centers (CoordinateUtils format: x0,y0,x1,y1). + final int[] codePoints = new int[] { 't', 'h' }; + final int[] coords = new int[] { 100, 200, 110, 210 }; + wordComposer.seedInputPointersFromKeyCenters(codePoints, coords); + + final InputPointers result = wordComposer.getInputPointers(); + assertEquals(2, result.getPointerSize()); + assertEquals(100, result.getXCoordinates()[0]); + assertEquals(200, result.getYCoordinates()[0]); + assertEquals(110, result.getXCoordinates()[1]); + assertEquals(210, result.getYCoordinates()[1]); + } + + // A key the layout can't resolve comes back as NOT_A_COORDINATE; feeding that to the + // recognizer as a real point would warp the stroke toward (-1,-1). The seed must skip it. + @Test + public void testSeedInputPointersSkipsUnresolvableKeys() { + final WordComposer wordComposer = new WordComposer(); + + final InputPointers stale = new InputPointers(16); + for (int i = 0; i < 5; i++) { + stale.addPointer(i, i, 0, i * 10); + } + wordComposer.setBatchInputPointers(stale); + + // 3-char word whose middle key has no geometry on the current layout. + final int[] codePoints = new int[] { 't', '\'', 'h' }; + final int[] coords = new int[] { 100, 200, + Constants.NOT_A_COORDINATE, Constants.NOT_A_COORDINATE, + 110, 210 }; + wordComposer.seedInputPointersFromKeyCenters(codePoints, coords); + + final InputPointers result = wordComposer.getInputPointers(); + assertEquals(2, result.getPointerSize()); + assertEquals(100, result.getXCoordinates()[0]); + assertEquals(200, result.getYCoordinates()[0]); + assertEquals(110, result.getXCoordinates()[1]); + assertEquals(210, result.getYCoordinates()[1]); + } + + // End-to-end simulation of the on-device "th ing backspace ing -> thinking" failure + // (fragment-pop stale stroke). Replays the exact WordComposer call sequence InputLogic + // makes: tap T,H -> arm extend base -> first gesture (24 pts, merged trail) -> + // gesture-end rebuild as "Thing" -> backspace fragment-pop ("Th" + unsetBatchMode + + // seed realignment) -> re-arm extend base -> second gesture (25 pts). The second + // gesture's merged stroke must be a densified base anchored at the TRUNCATED word's key + // centers (t,h) followed by the new gesture — NOT a trail carrying the popped "ing" + // fragment's geometry. (The base is now resampled to the swipe's density, so its exact + // point count is no longer a fixed 2; the invariant is that it is anchored at t/h, not the + // stale fragment.) If this passes but the device still misbehaves, the contamination lives + // outside WordComposer (threading / a caller repopulating the buffer). + @Test + public void testFragmentPopThenReswipeUsesSeededBaseNotStaleTrail() { + final WordComposer wordComposer = new WordComposer(); + + // Key centers for t and h (CoordinateUtils format). + final int T_X = 520, T_Y = 84, H_X = 637, H_Y = 230; + final int[] thCodePoints = new int[] { 't', 'h' }; + final int[] thCoords = new int[] { T_X, T_Y, H_X, H_Y }; + + // 1. Taps T, H — the non-batch applyProcessedEvent path records tap coords. + wordComposer.setComposingWord(thCodePoints, thCoords); + assertEquals(2, wordComposer.getInputPointers().getPointerSize()); + + // 2. First swipe starts: InputLogic#onStartBatchInput arms the merged-trail base + // from the composer's own pointers. + wordComposer.setExtendBatchInputBase(wordComposer.getInputPointers()); + assertEquals(2, wordComposer.getExtendBatchInputBaseSize()); + + // 3. First gesture ("ing"-shaped, 24 raw points) merges onto the base. + final InputPointers firstGesture = new InputPointers(32); + for (int i = 0; i < 24; i++) { + firstGesture.addPointer(700 + i * 4, 100 + i * 6, 0, 2000 + i * 16); + } + wordComposer.setBatchInputPointers(firstGesture); + // The 2 tap points are densified to the swipe's spacing before the 24-point gesture is + // appended, so the merged stroke is (densified base > 2) + 24. + final int afterFirstGesture = wordComposer.getInputPointers().getPointerSize(); + assertTrue("tapped prefix is densified beyond its 2 vertices", afterFirstGesture - 24 > 2); + + // 4. Gesture end (onUpdateTailBatchInputCompleted): clear the base, rebuild the + // composer text as the recognized word. Pointers are deliberately untouched. + wordComposer.setExtendBatchInputBase(null); + wordComposer.setBatchInputWord("Thing"); + assertEquals("setBatchInputWord must not touch the raw pointer buffer", + afterFirstGesture, wordComposer.getInputPointers().getPointerSize()); + assertTrue(wordComposer.isBatchMode()); + + // 5. Backspace fragment-pop (InputLogic#tryFragmentBackspace truncation branch): + // rebuild as the truncated word, leave batch mode, realign the stroke buffer + // to the truncated word's key centers. + wordComposer.setBatchInputWord("Th"); + wordComposer.unsetBatchMode(); + wordComposer.seedInputPointersFromKeyCenters(thCodePoints, thCoords); + assertEquals("after the pop the stroke buffer must hold ONLY the truncated word's" + + " key centers", 2, wordComposer.getInputPointers().getPointerSize()); + + // 6. Second swipe starts: re-arm the base from the (now seeded) pointers. + wordComposer.setExtendBatchInputBase(wordComposer.getInputPointers()); + assertEquals("the re-armed base must be the 2-point seed, not the stale 26-point" + + " pre-pop trail", 2, wordComposer.getExtendBatchInputBaseSize()); + + // 7. Second gesture (re-swiped "ing", 25 raw points). + final InputPointers secondGesture = new InputPointers(32); + for (int i = 0; i < 25; i++) { + secondGesture.addPointer(700 + i * 4, 100 + i * 6, 0, 9000 + i * 16); + } + wordComposer.setBatchInputPointers(secondGesture); + + final InputPointers merged = wordComposer.getInputPointers(); + final int[] xs = merged.getXCoordinates(); + final int[] ys = merged.getYCoordinates(); + final int total = merged.getPointerSize(); + // The seeded base is the sparse key-center representation (t,h are far apart relative to + // the swipe's spacing), so each center gets a small CLUSTER of samples before the 25-point + // gesture is appended: merged = (clustered base > 2) + 25. Assert the shape, and that the + // base is anchored at the t/h key centers (NOT carrying the popped "ing" fragment). + final int baseCount = total - 25; + assertTrue("seeded base must be clustered beyond its 2 key centers", baseCount > 2); + // Each key center gets a small cluster spread symmetrically a few px around it, so the + // cluster centroid stays ON the key. Assert a base sample lands near the 't' AND near the + // 'h' key center (within the cluster spread), and that none carry the stale "ing" fragment. + boolean anchoredAtT = false, anchoredAtH = false; + for (int i = 0; i < baseCount; i++) { + if (Math.abs(xs[i] - T_X) <= 16 && Math.abs(ys[i] - T_Y) <= 16) anchoredAtT = true; + if (Math.abs(xs[i] - H_X) <= 16 && Math.abs(ys[i] - H_Y) <= 16) anchoredAtH = true; + } + assertTrue("a base cluster must be anchored at the 't' key center", anchoredAtT); + assertTrue("a base cluster must be anchored at the 'h' key center", anchoredAtH); + assertEquals("the new gesture follows the clustered base", 700, xs[baseCount]); + // Times strictly increase across the whole synthetic stroke. + final int[] times = merged.getTimes(); + for (int i = 1; i < total; i++) { + assertTrue("times must strictly increase at index " + i, times[i] > times[i - 1]); + } + } + @Test public void testExtendBatchInputBaseEmptyIsNoOp() { final WordComposer wordComposer = new WordComposer(); diff --git a/docs/COMPOSING_WORD_SOURCE_OF_TRUTH.md b/docs/COMPOSING_WORD_SOURCE_OF_TRUTH.md new file mode 100644 index 000000000..697f99351 --- /dev/null +++ b/docs/COMPOSING_WORD_SOURCE_OF_TRUTH.md @@ -0,0 +1,223 @@ +# Composing word: editor text as the single source of truth (design) + +> Status: **partially shipped — see "Phases / outcome" below.** The drift-prone-stroke bug +> class this note set out to kill is fixed and on-device validated; the broader "retire all +> parallel state" refactor was *not* pursued in full (the targeted fix proved sufficient and +> the real captured stroke beat synthesized geometry on feel). This note is now the record of +> what shipped and why the plan changed, not a forward plan. +> +> Grounding: claims below were verified by read-only scouting of `InputLogic`, +> `WordComposer`, `RichInputConnection`. File/symbol refs are a snapshot — prefer the named +> symbols and re-confirm at implementation. + +## Problem + +Two-thumb / multi-part composition keeps several pieces of **gesture state in memory, +parallel to the editor text**, and they drift out of sync: + +- `WordComposer.mInputPointers` — the raw stroke buffer. Intentionally *not* cleared by + `reset()` / backspace (so a gesture commit can re-feed it via `setBatchInputWord`), and + `ResizableIntArray.addAt` overwrites in place **without shrinking the length**. +- `InputLogic.mLiveStroke` — the live-converge accumulator. +- `WordComposer.mExtendBatchInputBase` — the merged-trail base prepended to each new gesture. +- `InputLogic.mGestureFragmentBoundaries` — char-index boundaries for "delete last fragment". + +Because these are a *second* source of truth, anything that edits the text outside the +tracked flow — a backspace, a partial delete, a cursor move, autocorrect, an abnormal +gesture end — leaves them stale. That is the recurring bug class: + +- **Stale-stroke accumulation:** mis-recognize a multi-part word, backspace, retry → the + next swipe merges with leftover geometry and builds an ever-longer garbage word. (Partly + fixed by resetting `mInputPointers` on the first tap of a fresh word; the **fragment-pop** + path is still uncovered — see Phase 1.) +- **Fragment-backspace fragility:** `mGestureFragmentBoundaries` needs band-aid "filter + stale boundaries past current length" logic, only works in specific spacing configs, and + bails entirely when the cursor is in the middle of the word. +- **No mid-word editing:** you cannot move the cursor into the middle of a composing word, + or delete part of it, and continue building with gestures — the in-memory state assumes + append-at-end. + +## Core principle + +**The editor text + the composing region (and the cursor within it) are the single source +of truth for "the current word." Stroke geometry is derived from that text on demand, not +maintained as separate, drift-prone state.** + +A word's plausible stroke is just the sequence of its keys' centers — recoverable from the +*text* alone. So we never need to remember a stroke across edits; we can always reconstruct +one when re-recognition is needed. + +## Trigger model (what (re)composes, and what doesn't) + +**Re-recognition is the core feature (the Nintype behavior) and must be preserved:** +combining taps and swipes — before, during, or after each other — into one word that +re-resolves against the context before the cursor. The rules below govern only *when* it +fires, so a deliberate tap is never silently reshaped: + +- **A swipe always (re)composes, and always carries context.** A swipe at the cursor + re-recognizes, pulling in the word at the cursor as context — whether that word was typed, + swiped, one the cursor was just moved into, **or one already committed to the text box** + (re-entered via a cursor move). *Example: move to the end of "Doc" — composing OR a + committed word in the field — swipe "ument" → re-recognizes with the "Doc" context → + "Document".* This swipe-onto-any-existing-word capability is the full payoff (Phase 3). +- **A tap is exact UNLESS the current word already contains a swipe.** A tap into a word with + no swipe in it is literal — it never re-recognizes. *Example: move between "Do" and "c", + tap "g" → "Dogc", exactly.* But once the current word's composition includes a swipe, taps + before/during/after it **do** contribute and re-recognize — this is the combine-taps-and- + swipes feature, kept (today's `PREF_MULTIPART_RERECOGNIZE_TAPS`), just reimplemented on the + source-of-truth model. +- **Moving the cursor is context-only.** It makes the word at the cursor *known* (so a + following swipe can build on it) but never re-composes on its own. The next swipe + re-recognizes with that context; the next bare tap (no swipe in the word) stays exact. + +Net: **swipes drive re-recognition and always carry the surrounding-word context; taps stay +exact until a swipe is in play, then they contribute; cursor moves are inert until you act.** +Space remains the word-submission point. + +### Current behavior (pre-Phase 3) — how cursor moves actually behave today + +The Trigger model above is the *target*. Until Phase 3 lands, moving the cursor onto an +existing word behaves differently for swipe vs. tap, and this is worth knowing: + +- **Move the cursor into another word, or midway through a word, then SWIPE** → the keyboard + commits/clears the re-entered word, inserts an autospace, and **starts a brand-new word**. + It does *not* re-recognize the word the cursor is in. *Mechanism:* a cursor landing in the + front/middle of the composing word makes `isCursorFrontOrMiddleOfComposingWord()` true, so + `onStartBatchInput` takes the reset branch (`InputLogic.java` ~line 787); with no composing + word left to extend and a letter before the cursor, the autospace (`SpaceState.PHANTOM`) + branch fires (~line 846) and the gesture recognizes fresh. +- **Move the cursor into/onto a word, then TAP** → you **edit that existing word** with a + literal character at the cursor. *Mechanism:* `tryLiveConvergeTap` returns early on the same + front/middle guard (and on `!mCombiningWordHasGestureFragment` for a word with no swipe in + it), so the tap falls through to normal literal insertion. + +This is *not a bug* — it's a usable split: swipe = "start fresh here," tap = "edit what's +here." Phase 3 would have changed the swipe half (re-recognize the re-entered word with its +text as context — the "move to end of `Doc`, swipe `ument` → `Document`" payoff). + +> **Product decision (2026-06): keep this split.** The swipe = start-fresh / tap = edit +> behavior is the intended design, not a stepping stone. The swipe-onto-an-existing-word +> re-recognition described in the Trigger model is **descoped** — see Phase 3 below. + +## The foundation already exists + +HeliBoard already reconstructs a composing word from editor text when the cursor lands in a +word — `restartSuggestionsOnWordTouchedByCursor` → `getWordRangeAtCursor` → +`restartSuggestions`, whose core is: + +```java +mWordComposer.setComposingWord(codePoints, getCoordinatesForCurrentKeyboard(codePoints)); +mWordComposer.setCursorPositionWithinWord(...); +mConnection.setComposingRegion(start, end); +``` + +`getCoordinatesForCurrentKeyboard(text)` **synthesizes per-character key-center +coordinates from text alone** — exactly the "derive a stroke from the word" primitive this +design needs. Today it is used for **tap recorrection only**; it is never wired into the +gesture / live-converge path. The refactor is largely about routing two-thumb composition +through this existing machinery instead of around it. + +## Target design (original aspiration — only partially realized) + +> The bullets below are the *original* "remove all parallel state" vision. In practice only the +> stroke-from-text primitive was adopted, and only at the swipe-extend consumption point — see +> "Phases / outcome" above for what actually shipped. Kept here for context. + +- **The word being built = the composing region in the editor.** It can be (re)established + from any cursor position via `getWordRangeAtCursor` + `setComposingRegion`, which the + editor already maintains. +- **Swipe-on-word composition becomes (near) stateless.** When a *swipe* extends the word + at the cursor: take that word's *text* → synthesize its key-center stroke + (`getCoordinatesForCurrentKeyboard`) → append the swipe's raw points → re-recognize. No + `mLiveStroke` accumulator to leak. Taps never enter this path (see Trigger model). +- **Fragment boundaries become derived,** not stored. "Delete last fragment" is computed + from the text/recognition at delete time, so there is nothing to keep in sync. +- **Backspace, partial delete, and cursor moves stop being special cases.** Each just + changes the text (a cursor move is otherwise inert — context-only); the *next swipe* + rebuilds its base from whatever text is actually there. + +### State inventory — what actually happened +- **`mLiveStroke`: KEPT, UNCHANGED.** The plan was to retire it (derive the tap-re-recognition + prefix from text). An attempt to do so — and a follow-up "real stroke primary, text-derived + fallback" hybrid (`buildStrokeFromWordText`, `mComposingWordHasSwipeContent`) — was **reverted**: + the fallback turned out to be effectively dead code (the guards that admit a re-recognizing tap + are cleared on the same edits that empty the stroke, so it never fired), and the real captured + stroke re-recognizes better than synthesized key-centers anyway. The live-converge tap path + still uses `mLiveStroke` exactly as before. +- **`mInputPointers` drift: FIXED, not retired.** The "buffer survives reset / doesn't shrink on + delete" contract stays, but the drift it caused is now corrected **at the point of consumption** + rather than by removing the buffer (see Phases / outcome). `mGestureFragmentBoundaries` is also + still in use (not retired). +- **`mExtendBatchInputBase`** remains the mechanism for feeding "context prefix + new input" to + the recognizer. The prefix is the *real* stroke when it still matches the text, and is rebuilt + from text-derived key centers only when an edit made it stale. +- **Reused as planned:** `getCoordinatesForCurrentKeyboard` (text → key-center geometry) is the + one primitive from this plan that did get wired into the gesture path, via + `seedInputPointersFromKeyCenters` / `realignComposerStrokeToText`. + +## Phases / outcome (what was planned vs what shipped) + +1. **Stop the bleeding in fragment mode.** ✅ **SHIPPED & on-device validated.** On a + fragment-pop (`tryFragmentBackspace`), realign `mInputPointers` to the truncated word + (rebuild from its key centers via `seedInputPointersFromKeyCenters`) so a following + swipe-extend no longer merges with the pre-pop stroke. Lowercases the word before the + exact key lookup and skips `NOT_A_COORDINATE` keys. +2. **Re-express re-recognition from text (retire `mLiveStroke`).** ❌ **ATTEMPTED, then + REVERTED.** Two variants were tried: (a) replace `mLiveStroke` with text-derived geometry — + felt subpar, the real captured curves re-recognize better; (b) a "real primary + text-derived + fallback" hybrid (`buildStrokeFromWordText`, `mComposingWordHasSwipeContent`) — the fallback + was effectively unreachable (the guards that admit a re-recognizing tap are cleared by the + same edits that empty the stroke), so it changed nothing. Both were rolled back; `mLiveStroke` + stays as-is. **Lesson:** the parallel stroke state isn't worth removing for the tap path; it's + only a problem for the *swipe-extend base*, which is fixed surgically below. +3. **Generalize / swipe-onto-committed-word.** 🚫 **Descoped (2026-06).** Today's split — + **swipe at a moved cursor starts a fresh word, tap edits the existing word** — is the intended + behavior (product decision), so the Trigger model's "a swipe always re-recognizes the word at + the cursor" does *not* apply to a re-entered committed word; it governs only a word actively + being built at the cursor end. + +### What actually fixed the bug (the shipped general fix) + +Instead of removing the parallel stroke buffer, the drift is corrected **at the single point it +is consumed** — when a swipe arms its merged-trail extend base in `onStartBatchInput`: + +- `mComposingStrokeStale` is set on **any** backspace and cleared when a gesture rebuilds the + buffer (or the word ends). It means "the composing text was edited since the stroke last + matched it." +- At the extend-arm site, if stale, the base is rebuilt from the composing word's text via + `realignComposerStrokeToText` (→ `getCoordinatesForCurrentKeyboard` → + `seedInputPointersFromKeyCenters`); if not stale, the **real captured stroke** is used. +- This covers every edit path at once (single-char, fragment-pop, selection / multi-char delete, + cursor re-compose) without per-path patches, and leaves continuous swipe+swipe and tap+swipe + using their real stroke. `realignComposerStrokeToText` no-ops if the layout can't resolve any + key, so it never disarms the extend. **Shipped & on-device validated** (the recurring + "Thing→delete→Whining" failure is gone). + +### Next: Nintype-style whole-word backspace (planned, not yet built) + +A follow-up, independent of the above: in **"Whole word" backspace mode**, pop the last swipe +fragment while composing, then delete a **whole previous word** (any word, swiped or typed) once +past the composing word — gated by the existing `PREF_COMBINING_BACKSPACE_DELETES_COMPOSING_TEXT` +toggle (to be relabelled to describe this), with key-repeat deleting whole words. Requires +relaxing `tryFragmentBackspace`'s whole-word-mode bail and adding a word-boundary delete for +committed text (`getTextBeforeCursor` + `SpacingAndPunctuations`), gated to two-thumb mode so +plain typists are unaffected. + +## Risks / validation + +- **Synthesized strokes ≠ real strokes.** `getCoordinatesForCurrentKeyboard` yields key + *centers*, not the user's actual curves. Re-recognition from synthesized geometry may + differ from the captured stroke. **This is the primary thing to validate on-device** (the + recognizer may be robust to clean key-center input, or need tuning). Gate each phase on a + real-device check, conservative defaults, and the trace/replay harness where possible. +- **Performance:** re-synthesize + re-recognize per fragment is more work than appending to + a buffer, but the same order as today's live-converge per-tap recognition. +- **Feel:** mid-word continuation and derived fragment behavior are feel calls — ship as + tunable where it matters, decide by typing on it (per the project's feel-driven principle). + +## Why this is the right direction + +Every current bug here is the same disease — parallel gesture state drifting from the +editor. This refactor **removes** that parallel state rather than adding more patches, and +unlocks mid-word cursor editing and partial deletes for free. Each phase reduces state and +is shippable on its own, so the risk is bounded and the wins are incremental.