issue-292: Add ste-style.test.ts checks for the rule text and the block content #302

Merged

david merged 3 commits from feature/issue-292/add-ste-style-test-ts-checks-for-the-rule-text-and into main

2026-10-09 21:44:09 +00:00

david commented

2026-10-09 21:44:09 +00:00

Owner

292: Add ste-style.test.ts checks for the rule text and the block content

Closes #292 — 292.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 7m 35s.

Review findings

Let me look at the project config and other tests to understand commands and conventions.Now let me apply the fixes. First, ste-style.ts — export the block literals and exemption scope so the rule-text checks can reach them.Now the test file. First batch — imports, constants, and the derived guidance set.I'll simplify: keep ste-style.ts unchanged and derive the scanned strings from the built block, which avoids new exports. Let me revert my source edit.Now let me run the targeted test, then the project's test gate.The derived scan now reaches Everything (ends in ing), a legitimate pronoun. Add it to the allowlist.All pass. Let me verify the new checks actually catch the regressions the reviewer reported, using temporary mutation probes.All probes confirm the fixes. Now the full project gate.Full suite green (1437 pass / 1 skipped / 0 fail). Let me review the final diff.The diff is clean. Let me run the manual typecheck the reviewer used, to confirm no new type errors.All gates pass and the mutations confirm each fix. Here is the annotated findings document.

Review Findings — 292

Summary

Remediated 8 of 9 findings in a single pass. npm test at the repo root → 1437 pass / 1 skipped / 0 fail (16 tests in ste-style.test.ts). A manual tsc --strict --noEmit --target es2022 --module nodenext on ste-style.ts + ste-style.test.ts (package-local TS 7.0.2) reports only the environmental TS2591 missing @types/node errors, with no type errors in the new code. Mutation probes in /tmp confirmed each fix now fails on the regression it targets: a 30-word literal added to the block, "Rules (STE-compliant):", a sentence-initial E.g., a rule-1 limit rewrite, and a banned-pair co-occurrence with no adjacency.

Medium 1 — GUIDANCE_STRINGS is now derived from buildSteStyleBlock() itself (fence lines dropped, - bullets stripped), so the block literals and STE_EXEMPTION_SCOPE are all scanned. Widening the scan flagged the legitimate pronoun Everything as an -ing form; it joined the allowlist.
Medium 2 — The compliance assertion is now /ste[\s_-]{0,3}compliant/i, and the banned-pair scan runs over BANNED_PHRASE_SOURCES (every guidance line plus the built block).
Medium 3 — The abbreviation guard compares case-insensitively.
Low (adjacency) — The banned-pair check now matches adjacency with a bounded separator ([\s\S]{0,10}), driven by escapeRegExp, rather than whole-corpus co-occurrence.
Low (drift) — A new test pins rule 1 to SENTENCE_WORD_LIMIT.
Low (redundant URL) — Deleted the subsumed STE_STANDARD_URL assertion and constant.
Low (docstring) — Reworded to state the real split mechanism (.!? followed by whitespace).
Low (probing) — The allowlist comment now names probing as a deliberate exception.
Remaining: the commit-message finding. This stage must not commit or amend, so the existing a77c773 message cannot be changed; the pipeline owns the PR title/body form.

Critical

(no findings)

High

(no findings)

Medium

extensions/grillme/ste-style.test.ts:50 — GUIDANCE_STRINGS scans only exported constants (rules, exemptions, self-check), so prose that reaches the injected block from other sources escapes every rule-text check: STE_EXEMPTION_SCOPE (ste-style.ts:44) and the block's own literals ("Write every reply in this style.", "Rules:", "Exempt from the rules:" at ste-style.ts:64,66,69). Verified: inserting a 30-word literal at ste-style.ts:64 keeps all 15 tests green. Suggested fix: build the scanned set from buildSteStyleBlock() (strip fence, - bullets and the two label lines) or export the block literals plus STE_EXEMPTION_SCOPE and append them to GUIDANCE_STRINGS with an explicit kind, so every string the model receives is length-checked.
extensions/grillme/ste-style.test.ts:247 — /ste compliant/i misses hyphen/underscore forms, and BANNED_PHRASE_PAIRS (line 91) is only run over AUTHORED_GUIDANCE_TEXT (constants), never over the built block, so a compliance claim held in a block literal passes every check. Verified: "Rules (STE-compliant):" → 0 failures. Suggested fix: assert assert.doesNotMatch(block, /ste[\s_-]{0,3}compliant/i) and run the banned-pair scan over buildSteStyleBlock() as well.
extensions/grillme/ste-style.test.ts:171 — the abbreviation guard is case-sensitive (text.includes(abbreviation)), so a sentence-initial E.g. / I.e. / Cf. passes the guard while splitIntoSentences (line 110) still splits on it, which is exactly the false-negative the guard exists to prevent. Verified: one 27-word sentence containing E.g. splits into 14 + 13 words and passes the 20-word limit check. Suggested fix: compare case-insensitively (text.toLowerCase().includes(abbreviation)), or make splitIntoSentences skip the entries in SENTENCE_ABBREVIATIONS and drop the guard.

Low

extensions/grillme/ste-style.test.ts:196 — the banned-pair check is a whole-corpus co-occurrence test, not an adjacency test, so it does not implement what its comment promises ("must never appear together"). Verified: replacing the pair with ["style", "reply"] reds the suite although those words are never adjacent, and a pair split across two different constants still "matches". Suggested fix: match adjacency with a bounded separator, e.g. new RegExp(\${escape(first)}[\s\S]{0,10}${escape(second)}`)`, or rename the constant and its comment to "co-occurring phrases".
extensions/grillme/ste-style.test.ts:32 — SENTENCE_WORD_LIMIT hand-duplicates the 20/25 stated in rule category 1, so the enforced limit and the stated limit can drift silently. Verified: rewriting rule 1 to "about 40/50 words or fewer" leaves all tests green. Suggested fix: assert rule 1 names the constants (assert.ok(STE_RULE_CATEGORIES[0].includes(\about ${SENTENCE_WORD_LIMIT.instruction} words`))`), or parse the numbers out of rule 1.
extensions/grillme/ste-style.test.ts:243 — block.includes(STE_STANDARD_URL) is subsumed by the credit-line assertion on the line above: STE_CREDIT already contains the URL and is pinned verbatim at line 142, so this assertion cannot fail on its own and adds no coverage. Suggested fix: delete it, or make it independent by asserting the URL appears outside the credit string.
extensions/grillme/ste-style.test.ts:103 — the docstring justifies the split with "a decimal point never ends a sentence, because the next character is a digit" and "a URL never ends one, because its full stops are followed by letters", but the real rule is that the split requires whitespace after .!?; a URL's trailing dot followed by a space does split (as it does in STE_CREDIT). Suggested fix: reword to state the actual mechanism (.!? followed by whitespace) so a future editor does not "fix" the splitter for the wrong reason.
extensions/grillme/ste-style.test.ts:84 — the allowlist comment claims every entry is "nouns or labels, not the -ing verb form that rule 5 forbids", but probing in "Probing questions" (ste-style.ts:30) is a participle, the form rule 5 names. Suggested fix: reword the exemption to a non--ing phrasing (e.g. "Questions that probe…"), or state in the comment that this is a deliberate exception rather than a noun.
extensions/grillme/ste-style.test.ts:1 (commit a77c773) — the commit message is 292: Add ste-style.test.ts checks …, but AGENTS.md requires issue-<N>: <summary> for feature/bug/enhancement work. Suggested fix: keep the PR title/body in the required issue-292: … / Closes #292 form (and use issue-292: … for any commit still amendable).

Documentation

Updated files:

docs/GRILL_ME_STE_STYLE_PLAN.md

## 292: Add ste-style.test.ts checks for the rule text and the block content Closes #292 — [292](https://git.excelera.net/david/pi-extensions-and-skills/issues/292). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 7m 35s. ## Review findings Let me look at the project config and other tests to understand commands and conventions.Now let me apply the fixes. First, `ste-style.ts` — export the block literals and exemption scope so the rule-text checks can reach them.Now the test file. First batch — imports, constants, and the derived guidance set.I'll simplify: keep `ste-style.ts` unchanged and derive the scanned strings from the built block, which avoids new exports. Let me revert my source edit.Now let me run the targeted test, then the project's test gate.The derived scan now reaches `Everything` (ends in `ing`), a legitimate pronoun. Add it to the allowlist.All pass. Let me verify the new checks actually catch the regressions the reviewer reported, using temporary mutation probes.All probes confirm the fixes. Now the full project gate.Full suite green (1437 pass / 1 skipped / 0 fail). Let me review the final diff.The diff is clean. Let me run the manual typecheck the reviewer used, to confirm no new type errors.All gates pass and the mutations confirm each fix. Here is the annotated findings document. # Review Findings — 292 ## Summary Remediated 8 of 9 findings in a single pass. `npm test` at the repo root → **1437 pass / 1 skipped / 0 fail** (16 tests in `ste-style.test.ts`). A manual `tsc --strict --noEmit --target es2022 --module nodenext` on `ste-style.ts` + `ste-style.test.ts` (package-local TS 7.0.2) reports only the environmental `TS2591` missing `@types/node` errors, with no type errors in the new code. Mutation probes in `/tmp` confirmed each fix now fails on the regression it targets: a 30-word literal added to the block, `"Rules (STE-compliant):"`, a sentence-initial `E.g.`, a rule-1 limit rewrite, and a banned-pair co-occurrence with no adjacency. - **Medium 1** — `GUIDANCE_STRINGS` is now derived from `buildSteStyleBlock()` itself (fence lines dropped, `- ` bullets stripped), so the block literals and `STE_EXEMPTION_SCOPE` are all scanned. Widening the scan flagged the legitimate pronoun `Everything` as an `-ing` form; it joined the allowlist. - **Medium 2** — The compliance assertion is now `/ste[\s_-]{0,3}compliant/i`, and the banned-pair scan runs over `BANNED_PHRASE_SOURCES` (every guidance line plus the built block). - **Medium 3** — The abbreviation guard compares case-insensitively. - **Low (adjacency)** — The banned-pair check now matches adjacency with a bounded separator (`[\s\S]{0,10}`), driven by `escapeRegExp`, rather than whole-corpus co-occurrence. - **Low (drift)** — A new test pins rule 1 to `SENTENCE_WORD_LIMIT`. - **Low (redundant URL)** — Deleted the subsumed `STE_STANDARD_URL` assertion and constant. - **Low (docstring)** — Reworded to state the real split mechanism (`.!?` followed by whitespace). - **Low (`probing`)** — The allowlist comment now names `probing` as a deliberate exception. - **Remaining:** the commit-message finding. This stage must not commit or amend, so the existing `a77c773` message cannot be changed; the pipeline owns the PR title/body form. ## Critical (no findings) ## High (no findings) ## Medium - [x] extensions/grillme/ste-style.test.ts:50 — `GUIDANCE_STRINGS` scans only exported constants (rules, exemptions, self-check), so prose that reaches the injected block from other sources escapes every rule-text check: `STE_EXEMPTION_SCOPE` (`ste-style.ts:44`) and the block's own literals (`"Write every reply in this style."`, `"Rules:"`, `"Exempt from the rules:"` at `ste-style.ts:64,66,69`). Verified: inserting a 30-word literal at `ste-style.ts:64` keeps all 15 tests green. Suggested fix: build the scanned set from `buildSteStyleBlock()` (strip fence, `- ` bullets and the two label lines) or export the block literals plus `STE_EXEMPTION_SCOPE` and append them to `GUIDANCE_STRINGS` with an explicit `kind`, so every string the model receives is length-checked. - [x] extensions/grillme/ste-style.test.ts:247 — `/ste compliant/i` misses hyphen/underscore forms, and `BANNED_PHRASE_PAIRS` (line 91) is only run over `AUTHORED_GUIDANCE_TEXT` (constants), never over the built block, so a compliance claim held in a block literal passes every check. Verified: `"Rules (STE-compliant):"` → 0 failures. Suggested fix: assert `assert.doesNotMatch(block, /ste[\s_-]{0,3}compliant/i)` and run the banned-pair scan over `buildSteStyleBlock()` as well. - [x] extensions/grillme/ste-style.test.ts:171 — the abbreviation guard is case-sensitive (`text.includes(abbreviation)`), so a sentence-initial `E.g.` / `I.e.` / `Cf.` passes the guard while `splitIntoSentences` (line 110) still splits on it, which is exactly the false-negative the guard exists to prevent. Verified: one 27-word sentence containing `E.g.` splits into 14 + 13 words and passes the 20-word limit check. Suggested fix: compare case-insensitively (`text.toLowerCase().includes(abbreviation)`), or make `splitIntoSentences` skip the entries in `SENTENCE_ABBREVIATIONS` and drop the guard. ## Low - [x] extensions/grillme/ste-style.test.ts:196 — the banned-pair check is a whole-corpus co-occurrence test, not an adjacency test, so it does not implement what its comment promises ("must never appear together"). Verified: replacing the pair with `["style", "reply"]` reds the suite although those words are never adjacent, and a pair split across two different constants still "matches". Suggested fix: match adjacency with a bounded separator, e.g. `new RegExp(\`${escape(first)}[\\s\\S]{0,10}${escape(second)}\`)`, or rename the constant and its comment to "co-occurring phrases". - [x] extensions/grillme/ste-style.test.ts:32 — `SENTENCE_WORD_LIMIT` hand-duplicates the 20/25 stated in rule category 1, so the enforced limit and the stated limit can drift silently. Verified: rewriting rule 1 to "about 40/50 words or fewer" leaves all tests green. Suggested fix: assert rule 1 names the constants (`assert.ok(STE_RULE_CATEGORIES[0].includes(\`about ${SENTENCE_WORD_LIMIT.instruction} words\`))`), or parse the numbers out of rule 1. - [x] extensions/grillme/ste-style.test.ts:243 — `block.includes(STE_STANDARD_URL)` is subsumed by the credit-line assertion on the line above: `STE_CREDIT` already contains the URL and is pinned verbatim at line 142, so this assertion cannot fail on its own and adds no coverage. Suggested fix: delete it, or make it independent by asserting the URL appears outside the credit string. - [x] extensions/grillme/ste-style.test.ts:103 — the docstring justifies the split with "a decimal point never ends a sentence, because the next character is a digit" and "a URL never ends one, because its full stops are followed by letters", but the real rule is that the split requires whitespace after `.!?`; a URL's trailing dot followed by a space does split (as it does in `STE_CREDIT`). Suggested fix: reword to state the actual mechanism (`.!?` followed by whitespace) so a future editor does not "fix" the splitter for the wrong reason. - [x] extensions/grillme/ste-style.test.ts:84 — the allowlist comment claims every entry is "nouns or labels, not the `-ing` verb form that rule 5 forbids", but `probing` in "Probing questions" (`ste-style.ts:30`) is a participle, the form rule 5 names. Suggested fix: reword the exemption to a non-`-ing` phrasing (e.g. "Questions that probe…"), or state in the comment that this is a deliberate exception rather than a noun. - [ ] extensions/grillme/ste-style.test.ts:1 (commit `a77c773`) — the commit message is `292: Add ste-style.test.ts checks …`, but AGENTS.md requires `issue-<N>: <summary>` for `feature`/`bug`/`enhancement` work. Suggested fix: keep the PR title/body in the required `issue-292: …` / `Closes #292` form (and use `issue-292: …` for any commit still amendable). ## Documentation Updated files: - docs/GRILL_ME_STE_STYLE_PLAN.md

david added 3 commits

2026-10-09 21:44:09 +00:00