fix(mt#5052): Guard depth-request suppression against negated prompts with one proximity check - #3727
Conversation
… [no-deploy-impact]
Eight of the ten DEPTH_REQUEST_PATTERNS entries matched the negation of the
phrase they were calibrated for ("dont walk me through everything", "no need
to give me the full breakdown"), so the over-budget reminder was withheld
exactly when the principal asked for brevity. Measured at planning as latent
— 0 of 13 replayable suppressions in the live log carry a negator — and sized
as such.
One guard, not eight regex edits and not per-entry anchoring: a
proximity-bounded negation check (`isNegatedDepthRequest`) applied to every
occurrence of every entry inside `detectDepthRequest`. A match is rejected
when a negator ends within three tokens before the phrase in the same clause.
Anchoring was rejected on corpus evidence: 9 of the 13 live
`help-me-understand` suppressions are mid-sentence, and the anchored form
misses 4 of 6 live prompt shapes.
Tests pin both directions per entry, the mid-sentence live shapes, the tight
window ("I'm not sure I follow — help me understand …" still matches), and
the per-occurrence scan. `.claude/hooks` mirror regenerated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TPNyYTreM4F7PRbvh4xA5b
Minsky Reviewer StatusVerdict: APPROVED — no blocking findings Commands
|
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
The PR adds a single negation-guard mechanism at the detectDepthRequest seam and corresponding tests; the approach is coherent and largely aligned with the spec. However, two blocking issues were found. First, the negation regex permits any 0–3 intervening tokens, contradicting the spec’s “only filler allowed” constraint and causing genuine depth requests like “not only walk me through …” to be suppressed. Second, the clause-boundary regex omits ASCII hyphen/minus (‘-’), so common dash punctuation (e.g., “-” or “--”) won’t reset the clause, producing false negations after such dashes. I’ve also noted two non-blocking hardening/test gaps: guard against search() returning -1 when deriving phraseIndex, and add a unit test for the “not only …” construction to pin intended behavior. With these addressed, the PR should be ready to merge.
Findings
- [BLOCKING] .minsky/hooks/wall-of-text-detector.ts:1120 — Negation guard allows ANY intervening tokens (0–3), contradicting spec’s “only filler between negator and phrase” constraint
The implementation ofDEPTH_REQUEST_NEGATOR_REat.minsky/hooks/wall-of-text-detector.ts:~1120uses(?:\s+\S+){0,3}to permit up to 3 arbitrary tokens between the negator and the phrase start. The task spec (Success Criteria and PR description) calls for a proximity-bounded negation with only filler terms allowed between (e.g.,please,just,you,to). Allowing arbitrary tokens widens the guard beyond that contract and can suppress genuine requests. Example:"not only walk me through everything"is a positive request for depth (thenotscopes overonly, not the request), but the current regex will treat it as negated (negatornot+ tokenonlywithin 3 tokens), silently un-suppressing the reminder in the unsafe direction the spec warns about. Tighten the intervening-token allowance to a whitelist of permitted filler or otherwise exclude constructions like"not only ...", per the stated SC. - [BLOCKING] .minsky/hooks/wall-of-text-detector.ts:1130 — Missing common clause boundary: ASCII hyphen/minus ("-") and double hyphen (“--”) are not recognized as clause breaks, causing false negations
DEPTH_REQUEST_CLAUSE_BOUNDARY_REat.minsky/hooks/wall-of-text-detector.ts:~1130includes punctuation like commas and the Unicode en/em dashes (—/–), but omits the very common ASCII hyphen/minus-and--. Prompts often use-or--as a dash:"I'm not sure - help me understand …". Because-is not treated as a boundary, the slice forDEPTH_REQUEST_NEGATOR_REstill includesnot, and the(?:\s+\S+){0,3}\s*$tail can match"not sure -", incorrectly marking the subsequenthelp me understandas negated. Add-(and optionally--) to the clause-boundary regex to align behavior with the tested em dash and prevent over-suppression of genuine requests. - [NON-BLOCKING] .minsky/hooks/wall-of-text-detector.ts:1145 — Unprotected
m[0].search(/[a-z]/i)can return -1 if a future entry lacks ASCII letters; leads to a negativephraseIndex
IndetectDepthRequest, the phrase start is computed asconst phraseIndex = m.index + m[0].search(/[a-z]/i). If a futureDEPTH_REQUEST_PATTERNSentry were added that can match a segment with no ASCII letters before its core phrase (e.g., leading quotes/emoji/markup that fully consume the match, or a pattern starting with a non-ASCII letter),search()could return-1, yielding a negativephraseIndexand a mis-slice forisNegatedDepthRequest. While current entries all contain ASCII letters and tests pass, adding a guard for-1(fallback tom.index) would harden this seam against future pattern changes and avoid latent index bugs. - [NON-BLOCKING] .minsky/hooks/wall-of-text-depth-request.test.ts:160 — Missing test for the "not only … " construction — currently over-suppressed by the guard
Given the currentDEPTH_REQUEST_NEGATOR_REpermits arbitrary intervening tokens, a case like"not only walk me through everything"will be treated as a negated request even though it still asks for depth. Add a unit test that assertsdetectDepthRequest(["not only walk me through everything"]).matched === true(and similarly forgo-into-detail), or adjust it per the final guard semantics. This pins the intended behavior and prevents future regressions on this common English construction.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| The negated-request population in the live calibration log is counted and recorded — 0 confirmed of 13 replayable suppressions, 3 unrecoverable (planning, 2026-09-11). | Unverifiable | This evidence lives outside the repo in a state-dir JSONL log per the task text (e.g., ~/.local/state/minsky/projects/.../wall-of-text-calibration.jsonl). The diff cannot carry it; no in-repo artifact records the count. Per the Out-of-repo references rule, this cannot be verified from the diff. |
| Each of the eight entries stops matching its negated form (the table in Summary); none is left alone — the mechanism below covers all of them at once, so there is no per-entry reason to exempt one. | Met | .minsky/hooks/wall-of-text-depth-request.test.ts:120-186 adds AT1 asserting the eight negated prompts return matched: false. Implementation hooks this via the new negation guard used by detectDepthRequest (.minsky/hooks/wall-of-text-detector.ts:1102-1160) and scanning all occurrences (1161-1183). |
| The fix is ONE mechanism applied in detectDepthRequest, not eight hand-edited regexes and NOT per-entry imperative anchoring: a proximity-bounded negation guard … | Met | .minsky/hooks/wall-of-text-detector.ts:1102-1160 introduces DEPTH_REQUEST_NEGATOR_RE, DEPTH_REQUEST_CLAUSE_BOUNDARY_RE, and isNegatedDepthRequest(); detectDepthRequest now checks every match and rejects negated ones (1161-1183). No edits were made to the eight entry regexes. |
| The guard's own precision is pinned: a negator OUTSIDE the proximity window does not defeat a genuine request — “I'm not sure I follow — help me understand …” still matches. | Met | .minsky/hooks/wall-of-text-depth-request.test.ts:187-218 adds explicit tests for negators outside the window (dash-separated clause, comma boundary, four tokens) remaining matched. The clause boundary regex includes em/en dashes and commas (wall-of-text-detector.ts:1118-1126). |
| Every entry's measured phrase still matches with the same matchedPattern name; a unit test pins both directions per entry. | Met | .minsky/hooks/wall-of-text-depth-request.test.ts:143-159 covers genuine phrases for each entry and asserts matchedPattern equals the entry name. |
| bun scripts/measure-depth-request-widening.ts reports its window unchanged … (ADR-024 sign-off (b): 0 new false negatives on the calibration log). | Unverifiable | This requires running a script against a live calibration log. No script output is committed in the diff; cannot be verified from repo contents alone. |
| The negator list is recorded in the guard's docblock with the rule that it grows only on a calibration record that names the missed negator … and the shared primitive is noted against mt#4070. | Met | .minsky/hooks/wall-of-text-detector.ts:1088-1114 docblock enumerates the negators and records the evidence-before-expansion discipline; mt#4070 reference called out. The implementation exports the primitive at a single seam. |
Documentation impact
- no-update-needed — Internal detector logic and tests only. No CLI/API surface or documented behavior/signature changed (same exports and guard remain). I checked
.minsky/hooks/wall-of-text-detector.tsheader docs and inline comments were updated inline, and there are no separate docs under docs/ tied to the negation-guard semantics that would need updating.
…s bound a clause [no-deploy-impact]
Two blocking findings, both fixed at the same seam:
- A negator followed by `only` / `just` / `merely` / `simply` scopes a limiter,
not the request — "not only walk me through everything" and "don't just give
me the full breakdown" ask for the phrase AND more. A negative lookahead on
the negator excludes exactly that construction; the bounded window is
otherwise unchanged (the spec's own "no need to explain it in full detail"
needs a verb phrase between negator and entry, so a filler whitelist cannot
be the rule).
- A spaced ASCII hyphen or double hyphen is a clause boundary like the em-dash
("I'm not sure - help me understand"); an in-word hyphen (`deep-dive`) is not.
Non-blocking: `search()` returning -1 falls back to the match start; tests pin
the limiter and hyphen constructions and correct the one fixture that had
"don't just" as a negation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TPNyYTreM4F7PRbvh4xA5b
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Verified the R1 blocking concerns were addressed by the follow-up commit: the negation guard now excludes limiter scopes ("not only"/"don't just") and treats spaced ASCII hyphens as clause boundaries. The implementation introduces a single, proximity-bounded guard at detectDepthRequest’s seam and iterates all occurrences, with thorough unit tests covering the spec’s negated table, genuine phrases (including mid‑sentence shapes), window tightness, limiter exclusions, clause boundaries, and isNegatedDepthRequest as a testable seam. I found no new critical defects; one minor note suggests clarifying the $ anchoring-by-slice intent (non‑blocking). Overall, the changes meet the task’s success criteria that are verifiable from the repo, and this PR is ready to merge.
Findings
- [NON-BLOCKING] .minsky/hooks/wall-of-text-detector.ts:1065 — Anchored end-of-slice in
DEPTH_REQUEST_NEGATOR_RErelies on correct clause slicing — tests cover key cases, but consider an explicit comment about$anchoring semantics
DEPTH_REQUEST_NEGATOR_REends with\s*$andisNegatedDepthRequestapplies it tobefore.slice(clauseStart). This is correct (anchoring to the end of the clause-before-phrase), and your tests pin em-dash and spaced-hyphen clause boundaries. A short inline note that this use of$is intentional and depends on slicing to the clause would aid future maintainers (preventing a refactor that tests against the whole string). No change required for merge.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| The negated-request population in the live calibration log is counted and recorded — 0 confirmed of 13 replayable suppressions, 3 unrecoverable (planning, 2026-09-11). | Unverifiable | This measurement lives outside the repo (state-dir JSONL under ~/.local/state/minsky/projects/… per spec). No in-repo artifact to verify; cannot reproduce from diff. Treating as Unverifiable per out-of-repo reference. |
| Each of the eight entries stops matching its negated form (the table in ## Summary); none is left alone — the mechanism below covers all of them at once, so there is no per-entry reason to exempt one. | Met | .minsky/hooks/wall-of-text-depth-request.test.ts:157-183 — test "AT1 — each negated prompt from the spec's table is left unmatched" iterates the eight negated prompts and asserts detectDepthRequest returns matched=false for each. |
| The fix is ONE mechanism applied in detectDepthRequest, not eight hand-edited regexes and NOT per-entry imperative anchoring: a proximity-bounded negation guard … | Met | .minsky/hooks/wall-of-text-detector.ts:1026-1123 — introduces exported isNegatedDepthRequest() and DEPTH_REQUEST_NEGATOR_RE, and updates detectDepthRequest() to scan all occurrences and reject those negated by the guard. No per-entry regex edits in this diff. |
| The guard's own precision is pinned: a negator OUTSIDE the proximity window does not defeat a genuine request — "I'm not sure I follow — help me understand …" still matches … | Met | .minsky/hooks/wall-of-text-depth-request.test.ts:205-226 — test "the guard's window is tight — a negator outside it does not defeat a request" covers em-dash/spaced comma boundaries and >3-token separation, asserting matches remain true. |
| Every entry's measured phrase still matches with the same matchedPattern name; a unit test pins both directions per entry. | Met | .minsky/hooks/wall-of-text-depth-request.test.ts:186-203 — test "AT2 — each entry's calibrated phrase still matches, under the same name" asserts matched=true with the expected matchedPattern for all entries including mid-sentence shapes (see also 199-203). |
| bun scripts/measure-depth-request-widening.ts reports its window unchanged, so the guard un-suppresses none of the 13 live matches (ADR-024 sign-off (b): 0 new false negatives on the calibration log). | Unverifiable | This is a live script run against a local calibration log; outputs cited in PR description but not reproducible from repo-only diff. No changes to the script are in this PR to inspect. |
| The negator list is recorded in the guard's docblock with the rule that it grows only on a calibration record that names the missed negator — the same evidence-before-expansion discipline the list's narrowness note already states — and the shared primitive is noted against mt#4070. | Met | .minsky/hooks/wall-of-text-detector.ts:1029-1062 — docblock above DEPTH_REQUEST_NEGATOR_RE lists negators, states the evidence-before-expansion rule, and references mt#4070’s shared-primitive rationale. |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| isNegatedDepthRequest | function | .minsky/hooks/wall-of-text-depth-request.test.ts:144-151 — imported and exercised directly in the mt#5052 test block | Adopted | Exported from .minsky/hooks/wall-of-text-detector.ts for testability; consumed by the new tests only (no production callers expected). |
Documentation impact
- no-update-needed — Internal hook logic change with added unit tests; no user-facing CLI, API, or documented behavior modified. The detector remains live with the same surface and configuration. No docs in-repo reference the internal negation guard semantics.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Re-verified the R1 blocking concerns against the current diff: the negation guard now cleanly scopes to the preceding clause, excludes limiter constructions ("not only"/"don't just"), treats spaced ASCII hyphens as boundaries, and iterates all occurrences so a later genuine request is still recognized. Unit tests comprehensively pin the eight negated forms (AT1), preservation of genuine phrases and mid‑sentence shapes (AT2), tight window behavior, limiter exclusions, clause-boundary handling, and the testable isNegatedDepthRequest seam. I found no new critical defects introduced by this fix commit; the regex flag handling and matchAll usage are sound for the listed patterns. Event: APPROVE.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| The negated-request population in the live calibration log is counted and recorded — 0 confirmed of 13 replayable suppressions, 3 unrecoverable (planning, 2026-09-11). | Unverifiable | Lives outside the repo (state-dir JSONL under ~/.local/state/minsky/projects/, per spec). No artifact in the diff to verify; cannot reproduce from repo-only review. |
| Each of the eight entries stops matching its negated form (the table in ## Summary); none is left alone — the mechanism below covers all of them at once, so there is no per-entry reason to exempt one. | Met | .minsky/hooks/wall-of-text-depth-request.test.ts:157-183 — test "AT1 — each negated prompt from the spec's table is left unmatched" asserts detectDepthRequest returns matched=false for all eight negated prompts. |
| The fix is ONE mechanism applied in detectDepthRequest, not eight hand-edited regexes and NOT per-entry imperative anchoring: a proximity-bounded negation guard … | Met | .minsky/hooks/wall-of-text-detector.ts:1026-1123 — introduces DEPTH_REQUEST_NEGATOR_RE and isNegatedDepthRequest(), and updates detectDepthRequest() to scan all occurrences and reject negated ones; no per-entry regex edits are present. |
| The guard's own precision is pinned: a negator OUTSIDE the proximity window does not defeat a genuine request — "I'm not sure I follow — help me understand …" still matches … | Met | .minsky/hooks/wall-of-text-depth-request.test.ts:205-226 — covers em-dash/spaced-comma clause boundaries and >3-token separation; matches remain true as expected. |
| Every entry's measured phrase still matches with the same matchedPattern name; a unit test pins both directions per entry. | Met | .minsky/hooks/wall-of-text-depth-request.test.ts:186-203 — "AT2 — each entry's calibrated phrase still matches, under the same name" including mid‑sentence shapes. |
| bun scripts/measure-depth-request-widening.ts reports its window unchanged, so the guard un-suppresses none of the 13 live matches (ADR-024 sign-off (b): 0 new false negatives on the calibration log). | Unverifiable | Live script run against a local calibration log; outputs are cited in PR body but not reproducible from the diff. No changes to the script in this PR to inspect. |
| The negator list is recorded in the guard's docblock with the rule that it grows only on a calibration record that names the missed negator — the same evidence-before-expansion discipline the list's narrowness note already states — and the shared primitive is noted against mt#4070. | Met | .minsky/hooks/wall-of-text-detector.ts:1029-1062 — docblock enumerates negators, states the evidence-before-expansion rule, and references mt#4070’s drift argument. |
Adoption sweep
| Symbol | Kind | Consumers found | Classification | Notes |
|---|---|---|---|---|
| isNegatedDepthRequest | function | .minsky/hooks/wall-of-text-depth-request.test.ts:147 — imported and exercised in mt#5052 tests, /.claude/hooks/wall-of-text-detector.ts — exported for parity (no external consumers expected) | Adopted | Exported for testability; only unit tests consume it. No production wiring required by the spec. |
Documentation impact
- no-update-needed — Change is an internal guard addition to the wall-of-text detector with accompanying unit tests. No CLI, API, or user-facing behavior or configuration changed beyond internal precision, and no in-repo docs describe the negation-guard mechanism. Existing docs about the detector’s live posture (mt#3112) remain accurate.
Summary
wall-of-text-detector'sDEPTH_REQUEST_PATTERNSwithholds the over-budget reminder when the principal recently asked for depth. Eight of its ten entries also matched the NEGATION of their phrase — "dont walk me through everything, just the summary", "no need to give me the full breakdown", "please do not go into more detail" — so the reminder was suppressed exactly when the principal asked for brevity (the unsafe direction by the list's own narrowness note). Verified at planning: all 8 returnmatched: trueon the shipped list.Measured in the live calibration log (174 records since 2026-08-30, the only copy on disk): 16 depth-suppressed records, 13 replayable through the hook's own window logic, all 13
help-me-understand, 0 negated — so this ships as a latent robustness fix, sized accordingly.Key changes
.minsky/hooks/wall-of-text-detector.ts—isNegatedDepthRequest(text, index)+DEPTH_REQUEST_NEGATOR_RE: a match is rejected when a negator (don't/dont/do not/no need to/never/not/rather than/instead of/without) ends within three tokens before the phrase in the same clause (boundaries:,;:.!?()—–, newline, and contrastivebut).detectDepthRequestnow scans every occurrence of every entry, so a negated first mention does not hide a later genuine request. One mechanism at the seam, not eight regex edits (mt#4070's drift argument), and not per-entry anchoring liketell-me-more: 9 of the 13 live suppressions are mid-sentence ("…and also, help me understand…", "proceed, but first help me understand…") and the anchored form misses 4 of 6 live prompt shapes — anchoring would un-suppress most genuine requests, which is the friction incident mt#3112 exists to prevent. The negator list grows only on a calibration record naming a missed negator, per the list's own evidence-before-expansion discipline (docblock)..minsky/hooks/wall-of-text-depth-request.test.ts—mt#5052describe block, 7 tests..claude/hooks/wall-of-text-detector.ts— regenerated mirror.Testing
Spec ATs use the task's own numbering.
Execution evidence:
AT1 — the spec's eight negated prompts return
matched: false(AT1 — each negated prompt from the spec's table is left unmatched).AT2 — every entry's calibrated phrase still matches under the same name, plus the mid-sentence live shapes and the benign-negator shape "I'm not sure I follow — help me understand the peez thing" (
AT2 — …×2,the guard's window is tight …).AT3 —
bun scripts/measure-depth-request-widening.ts --log <state-dir>/wall-of-text-calibration.jsonl→window: head -120 -> 120 records; 75 injected / 45 suppressed,AT1 — newly suppressed: 3 (expected exactly 3),PASS — AT1, AT2, AT3 hold on this window. (The script exercises the entries directly, so this proves the list is untouched; the guard's effect on the live matches is the planning replay — 0 of 13 negated — plus the mid-sentence fixtures above.)AT4 —
bun scripts/run-guard-canaries.ts→[PASS] wall-of-text-detector (registry, expects=calibration);Total: 75 Passed: 73 Failed: 0 Missing: 2(the two missing are pre-existing, unrelated to this guard).AT5 — the negative control below.
validate_typecheck(session): 0 errors across 8 projects. ESLint on the two source files at--max-warnings=0: clean.bun run format:check: clean.Negative control: with the guard disabled in place (
if (true || !isNegatedDepthRequest(…))) and the new tests kept:The AT2 tests (genuine phrases, mid-sentence shapes) pass in both states, as pinned; restored afterwards (
grep -c "NEGATIVE CONTROL"→ 0, 169/169).[no-deploy-impact]—isDeploySurfaceFilereturnsfalsefor all three changed files (run over the diff, not recalled).Not in this PR
The
.codex/hooksmirror (PR #3253 makes it a compile output); PR #3412's disjoint edits to the same file (an import andmain()'s catch) — no conflict with this region.🤖 Generated with Claude Code
https://claude.ai/code/session_01TPNyYTreM4F7PRbvh4xA5b