Skip to content

fix(hooks): reject non-ASCII lead-in text in SatisfactionCapture rating guards - #1703

Open
takanorinishida wants to merge 1 commit into
danielmiessler:mainfrom
takanorinishida:fix/satisfaction-rating-ascii-guard
Open

fix(hooks): reject non-ASCII lead-in text in SatisfactionCapture rating guards#1703
takanorinishida wants to merge 1 commit into
danielmiessler:mainfrom
takanorinishida:fix/satisfaction-rating-ascii-guard

Conversation

@takanorinishida

Copy link
Copy Markdown

What's broken

parseExplicitRating() in hooks/SatisfactionCapture.hook.ts has two guards that reject a leading digit when it's followed by descriptive text (e.g. "2/10 items done" is correctly rejected as "not a rating"). Both guards only check ASCII character classes, so non-English descriptive text falls through and gets misread as an explicit rating.

Observed in the wild on a non-English (Japanese) install: the prompt "2はあとで" ("item 2, later") matched the generic rating pattern and was recorded as an explicit rating of 2, which triggered the rating < 5 low-rating capture path and wrote a spurious LEARNING/ALGORITHM synthesis note from a false signal. The same shape hits the N/10 fraction parser ("2/10件 完了" → misread as a 2/10 rating).

This isn't platform-specific — the hook has no macOS/Linux dependency — it affects any non-English (or more precisely, non-ASCII-script) prompt.

Fix

Two lines. Both guards now also reject when the trailing text starts with a non-ASCII character:

// generic parser
if (afterNumber.length > 0 && /^[/.\dA-Za-z]|^[^\x00-\x7F]/.test(afterNumber)) return null;

// N/10 fraction parser
if (fRest && (SENTENCE_STARTERS.test(fRest) || /^[^\x00-\x7F]/.test(fRest))) return null;

Why this doesn't change English behavior

The new branch only fires when the trailing text starts with a character outside \x00-\x7F. English descriptive text after a digit is ASCII by construction, so the branch never fires for English prompts — verified below.

Verification

Ran an isolated-LIFEOS_DIR probe harness (temp dir, no real ratings file touched) against both the pre-fix and post-fix hook, feeding the same 6 prompts to each:

Before (bug reproduction):

"8"                → rating 8
"9/10 nice"        → rating 9, comment "nice"
"2/10 items done"  → rejected (unchanged)
"8 素晴らしい"      → rating 8, comment "素晴らしい"
"2はあとで"         → rating 2   ← false positive
"2/10件 完了"       → rating 2   ← false positive

After:

"8"                → rating 8       (identical)
"9/10 nice"        → rating 9       (identical)
"2/10 items done"  → rejected       (identical)
"8 素晴らしい"      → rating 8       (identical)
"2はあとで"         → rejected       ← fixed
"2/10件 完了"       → rejected       ← fixed

All 4 English cases produce byte-identical output before/after. Only the 2 non-ASCII false-positive cases change, and they change from "silently corrupts a learning signal" to "correctly ignored."

Every deletion in this diff is a line replaced by an extended version of itself (same early-return, wider condition) — no behavior removed, only a false-positive path closed.

🤖 Generated with Claude Code

…ng guards

parseExplicitRating()'s two numeric-rating guards only rejected ASCII
descriptive text after a leading digit (e.g. "2/10 items done", "2 files
left"), so non-English prose fell through and got misread as a rating.

Observed in the wild: the Japanese prompt "2はあとで" ("item 2, later")
matched the generic rating pattern and was recorded as an explicit rating
of 2, triggering the low-rating capture path (rating < 5) and writing a
LEARNING/ALGORITHM synthesis note from a false signal. Same shape hits the
N/10 fraction parser ("2/10件 完了" → misread as 2/10).

Extend both guards to also reject when the trailing text starts with a
non-ASCII character. English prompts are unaffected — English descriptive
text after a digit is ASCII by construction, so the new branch never
fires for them; verified word-for-word against the pre-fix behavior with
an isolated LIFEOS_DIR probe harness (5 English cases identical, 2
Japanese false-positive cases now correctly rejected).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant