Skip to content

fix(mt#5052): Guard depth-request suppression against negated prompts with one proximity check - #3727

Merged
edobry merged 2 commits into
mainfrom
task/mt-5052
Sep 11, 2026
Merged

edobry merged 2 commits into
mainfrom
task/mt-5052

Conversation

@minsky-ai

@minsky-ai minsky-ai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary

wall-of-text-detector's DEPTH_REQUEST_PATTERNS withholds 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 return matched: true on 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 contrastive but). detectDepthRequest now 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 like tell-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#5052 describe 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.

    $ bun test --preload ./tests/setup.ts --timeout=15000 ./.minsky/hooks/wall-of-text-depth-request.test.ts ./.minsky/hooks/wall-of-text-detector.test.ts ./.minsky/hooks/wall-of-text-turn-window.test.ts
     169 pass
     0 fail
    Ran 169 tests across 3 files. [80.00ms]
    $ bun scripts/run-related-tests.ts .minsky/hooks/wall-of-text-detector.ts
    Ran 2899 tests across 61 files. [23.90s]
    run-related-tests.ts: 61 related test file(s) passed
    

    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:

(fail) mt#5052 — negated depth requests do not suppress > AT1 — each negated prompt from the spec's table is left unmatched
(fail) mt#5052 — negated depth requests do not suppress > the guard's window is tight — a negator outside it does not defeat a request
(fail) mt#5052 — negated depth requests do not suppress > the guard reaches a negator up to three tokens before the phrase
 11 pass
 3 fail
Ran 14 tests across 1 file.

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] — isDeploySurfaceFile returns false for all three changed files (run over the diff, not recalled).

Not in this PR

The .codex/hooks mirror (PR #3253 makes it a compile output); PR #3412's disjoint edits to the same file (an import and main()'s catch) — no conflict with this region.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TPNyYTreM4F7PRbvh4xA5b

… [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-ai minsky-ai Bot added the authorship/co-authored Co-authored by human and AI agent label Sep 11, 2026
@minsky-reviewer

minsky-reviewer Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 193K prompt, 6K completion | Duration: 64s
Mode: normal

Commands

  • /review — request a fresh review

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 of DEPTH_REQUEST_NEGATOR_RE at .minsky/hooks/wall-of-text-detector.ts:~1120 uses (?:\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 (the not scopes over only, not the request), but the current regex will treat it as negated (negator not + token only within 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_RE at .minsky/hooks/wall-of-text-detector.ts:~1130 includes 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 for DEPTH_REQUEST_NEGATOR_RE still includes not, and the (?:\s+\S+){0,3}\s*$ tail can match "not sure -", incorrectly marking the subsequent help me understand as 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 negative phraseIndex
    In detectDepthRequest, the phrase start is computed as const phraseIndex = m.index + m[0].search(/[a-z]/i). If a future DEPTH_REQUEST_PATTERNS entry 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 negative phraseIndex and a mis-slice for isNegatedDepthRequest. While current entries all contain ASCII letters and tests pass, adding a guard for -1 (fallback to m.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 current DEPTH_REQUEST_NEGATOR_RE permits 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 asserts detectDepthRequest(["not only walk me through everything"]).matched === true (and similarly for go-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.ts header 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

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_RE relies on correct clause slicing — tests cover key cases, but consider an explicit comment about $ anchoring semantics
    DEPTH_REQUEST_NEGATOR_RE ends with \s*$ and isNegatedDepthRequest applies it to before.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.

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@edobry
edobry merged commit 2c3edaa into main Sep 11, 2026
14 checks passed
@edobry
edobry deleted the task/mt-5052 branch September 11, 2026 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

authorship/co-authored Co-authored by human and AI agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant