issue-295: Rewrite the Grill Me command output and read-only block reasons #305

Merged

david merged 2 commits from feature/issue-295/rewrite-the-grill-me-command-output-and-read-only into main

2026-10-09 22:10:37 +00:00

david commented

2026-10-09 22:10:36 +00:00

Owner

295: Rewrite the Grill Me command output and read-only block reasons

Closes #295 — 295.

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 10m 12s.

Review findings

Now let me create the shared helper module:Now update command-output.test.ts:Now update tool-metadata.test.ts:Now update ste-style.test.ts:Now the index.ts fixes:Let me empirically verify the Medium 1 fix now catches a regression, on a throwaway copy:All findings fixed and the full suite is green (npm test: 1469 tests, 1468 pass, 1 pre-existing skip, 0 fail; cd extensions/grillme && node --test: 466 pass).

Review Findings — 295

Summary

All six findings are resolved in one pass. Medium: the AC1 check now asserts against HELP_CONTENT (verified on a scratch copy that deleting the - /grill <topic> help line now fails the suite), the /grill <topic> start message was rewritten to short simple-tense sentences and the suite now scans pi.sendUserMessage(...), and the duplicated parsing/STE helpers were moved into a new dependency-free extensions/grillme/ste-rule-checks.ts imported by all three suites. Low: the bash refusal reason now uses the same approval clause as the edit/write reason, the status overlay shows - Style: thorough default again, and the seeded checkpoint prose uses simple tense. No findings remain.

Critical

High

Medium

extensions/grillme/command-output.test.ts:256 — The AC1 check ("every documented command form byte-identical") asserts each form against the whole source file (INDEX_SOURCE.includes(form)) instead of the help text it guards. Verified on a scratch copy: deleting the - /grill <topic> line from the /grill help content leaves the suite fully green, because the same string still occurs in three tool-result messages (index.ts:534, index.ts:581, index.ts:635). The help list can therefore lose a documented form while AC1 still "passes". Suggested fix: assert HELP_CONTENT.includes(form) (the helper already exists at command-output.test.ts:171), keeping a whole-source check only where a form legitimately appears elsewhere.
extensions/grillme/index.ts:300 — The /grill <topic> start message is authored command output the user reads, and it was not rewritten: it still carries a 31-word sentence ("Begin by updating the checkpoint if needed, … ask the first focused Socratic question."), a 23-word sentence with producing, and the -ing form updating — rules 1, 4 and 5 of the same rule categories the issue calls the specification. The new suite scans only ctx.ui.notify(, so nothing fails. Suggested fix: split the message into short sentences ("Call grill_set_alternatives. Show 2-5 concrete answer choices in chat. Then ask the first question.") and extend notifyStrings()/authoredStrings() to also collect the first argument of each pi.sendUserMessage( call.
extensions/grillme/command-output.test.ts:68 — The parsing/STE helpers (SENTENCE_WORD_LIMIT, skipStringLiteral, balancedBlock, readLiteralAfter, splitIntoSentences, countWords, wordsIn) are now the third near-verbatim copy, alongside tool-metadata.test.ts:29 and ste-style.test.ts:29. A rule or splitter change (e.g. teaching the splitter to skip an abbreviation) must be applied in three places, and the suites can silently drift from each other. Suggested fix: move them into one dependency-free helper module, e.g. extensions/grillme/ste-rule-checks.ts, named so it does not match the *.test.ts / test-* run patterns, and import it from all three suites.

Low

extensions/grillme/index.ts:823 — The bash reason names the approval step "Get output approval", while the edit/write reason at index.ts:814 says "Wait for the user to approve a concrete output plan". Two terms for one meaning contradicts rule 6 ("Use one word for one meaning"), and the two reasons are meant to teach the same recovery path. Suggested fix: use the identical clause in both, e.g. "Wait for the user to approve a concrete output plan." in the bash reason too.
extensions/grillme/index.ts:150 — - Style: thorough drops the word "default", so the status overlay now disagrees with the prompt line for the same session (prompt.ts:182 renders - Grilling style: thorough default) and with README.md:40 / DESIGN.md:15 ("one thorough default grilling style"). The removal is not required by any rule category, and "thorough default" is a two-word group (rule 8 permits up to three). Suggested fix: restore - Style: thorough default, or align prompt.ts and the docs in the same change.
extensions/grillme/index.ts:88 — The seeded checkpoint prose (user-facing via /checkpoint) still uses continuous tense and -ing forms: "We are starting a grill-me session to reach shared understanding before producing outputs or implementation work." Rules 4 and 5 forbid both where a simple tense works. This string is outside the issue's enumerated list, so treat it as optional polish. Suggested fix: "We start this Grill Me session to reach shared understanding before we produce outputs or implementation work."

## 295: Rewrite the Grill Me command output and read-only block reasons Closes #295 — [295](https://git.excelera.net/david/pi-extensions-and-skills/issues/295). > 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 10m 12s. ## Review findings Now let me create the shared helper module:Now update `command-output.test.ts`:Now update `tool-metadata.test.ts`:Now update `ste-style.test.ts`:Now the `index.ts` fixes:Let me empirically verify the Medium 1 fix now catches a regression, on a throwaway copy:All findings fixed and the full suite is green (`npm test`: 1469 tests, 1468 pass, 1 pre-existing skip, 0 fail; `cd extensions/grillme && node --test`: 466 pass). # Review Findings — 295 ## Summary All six findings are resolved in one pass. Medium: the AC1 check now asserts against `HELP_CONTENT` (verified on a scratch copy that deleting the `- /grill <topic>` help line now fails the suite), the `/grill <topic>` start message was rewritten to short simple-tense sentences and the suite now scans `pi.sendUserMessage(...)`, and the duplicated parsing/STE helpers were moved into a new dependency-free `extensions/grillme/ste-rule-checks.ts` imported by all three suites. Low: the bash refusal reason now uses the same approval clause as the edit/write reason, the status overlay shows `- Style: thorough default` again, and the seeded checkpoint prose uses simple tense. No findings remain. ## Critical ## High ## Medium - [x] extensions/grillme/command-output.test.ts:256 — The AC1 check ("every documented command form byte-identical") asserts each form against the whole source file (`INDEX_SOURCE.includes(form)`) instead of the help text it guards. Verified on a scratch copy: deleting the `- /grill <topic>` line from the `/grill help` content leaves the suite fully green, because the same string still occurs in three tool-result messages (`index.ts:534`, `index.ts:581`, `index.ts:635`). The help list can therefore lose a documented form while AC1 still "passes". Suggested fix: assert `HELP_CONTENT.includes(form)` (the helper already exists at `command-output.test.ts:171`), keeping a whole-source check only where a form legitimately appears elsewhere. - [x] extensions/grillme/index.ts:300 — The `/grill <topic>` start message is authored command output the user reads, and it was not rewritten: it still carries a 31-word sentence ("Begin by updating the checkpoint if needed, … ask the first focused Socratic question."), a 23-word sentence with `producing`, and the `-ing` form `updating` — rules 1, 4 and 5 of the same rule categories the issue calls the specification. The new suite scans only `ctx.ui.notify(`, so nothing fails. Suggested fix: split the message into short sentences ("Call grill_set_alternatives. Show 2-5 concrete answer choices in chat. Then ask the first question.") and extend `notifyStrings()`/`authoredStrings()` to also collect the first argument of each `pi.sendUserMessage(` call. - [x] extensions/grillme/command-output.test.ts:68 — The parsing/STE helpers (`SENTENCE_WORD_LIMIT`, `skipStringLiteral`, `balancedBlock`, `readLiteralAfter`, `splitIntoSentences`, `countWords`, `wordsIn`) are now the third near-verbatim copy, alongside `tool-metadata.test.ts:29` and `ste-style.test.ts:29`. A rule or splitter change (e.g. teaching the splitter to skip an abbreviation) must be applied in three places, and the suites can silently drift from each other. Suggested fix: move them into one dependency-free helper module, e.g. `extensions/grillme/ste-rule-checks.ts`, named so it does not match the `*.test.ts` / `test-*` run patterns, and import it from all three suites. ## Low - [x] extensions/grillme/index.ts:823 — The bash reason names the approval step "Get output approval", while the edit/write reason at `index.ts:814` says "Wait for the user to approve a concrete output plan". Two terms for one meaning contradicts rule 6 ("Use one word for one meaning"), and the two reasons are meant to teach the same recovery path. Suggested fix: use the identical clause in both, e.g. "Wait for the user to approve a concrete output plan." in the bash reason too. - [x] extensions/grillme/index.ts:150 — `- Style: thorough` drops the word "default", so the status overlay now disagrees with the prompt line for the same session (`prompt.ts:182` renders `- Grilling style: thorough default`) and with `README.md:40` / `DESIGN.md:15` ("one thorough default grilling style"). The removal is not required by any rule category, and "thorough default" is a two-word group (rule 8 permits up to three). Suggested fix: restore `- Style: thorough default`, or align `prompt.ts` and the docs in the same change. - [x] extensions/grillme/index.ts:88 — The seeded checkpoint prose (user-facing via `/checkpoint`) still uses continuous tense and `-ing` forms: "We are starting a grill-me session to reach shared understanding before producing outputs or implementation work." Rules 4 and 5 forbid both where a simple tense works. This string is outside the issue's enumerated list, so treat it as optional polish. Suggested fix: "We start this Grill Me session to reach shared understanding before we produce outputs or implementation work."

david added 2 commits

2026-10-09 22:10:36 +00:00