issue-293: Extract buildGrillPrompt() and add the prompt-integration test #303

Merged

david merged 3 commits from feature/issue-293/extract-buildgrillprompt-and-add-the-prompt-integr into main

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

david commented

2026-10-09 21:50:08 +00:00

Owner

293: Extract buildGrillPrompt() and add the prompt-integration test

Closes #293 — 293.

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 5m 57s.

Review findings

Now let me apply the fixes. Starting with the README typecheck line (Medium 1):Now the type-consolidation fix (Medium 2) and moving the state→input mapping into prompt.ts (Medium 3). First prompt.ts:Now index.ts — replace the duplicated type declarations with type-only imports and use the moved mapper:Now rewrite the test file to (a) reword the header comment and (b) cover the untested branches and the state→input seam:Now let's run the grillme tests and the documented typecheck:All 15 pass. Now the full project gate:Full suite green (1452 pass / 0 fail / 1 unrelated skip). Let me verify the module graph and review the full diff:All gates pass and the module graph loads cleanly. Here are the annotated findings.

Review Findings — 293

Summary

Fixed all Medium findings and the Low documentation/comment finding. In extensions/grillme/README.md, the typecheck module inventory now includes prompt.ts. In extensions/grillme/prompt.ts, the duplicated GrillPrompt* unions are now the single source of truth — index.ts drops its local Intent/ResearchMode/GrillPhase/GrillAlternative declarations in favour of type-only aliased imports. The state→prompt mapper moved into prompt.ts as toGrillPromptInput() (called by index.ts), and prompt.test.ts now covers the phase branches (interview/output-selection/output), all three research modes, the output-preference fallback, and the state→input seam. The prompt.test.ts header comment was reworded to describe the real invariant. Gates: node --test extensions/grillme/*.test.ts passed (450 pass / 0 fail), npm test passed (1452 pass / 0 fail / 1 unrelated skip), and the documented tsc --noEmit … index.ts bash-filter.ts prompt.ts ste-style.ts widget.ts typecheck passed. Left unresolved: the AGENTS.md:26 commit-message finding, which is an MR-creation-time action and cannot be fixed without committing/amending, which this stage forbids.

Critical

High

Medium

extensions/grillme/README.md:90 — the documented typecheck line enumerates the extension's modules (index.ts bash-filter.ts ste-style.ts widget.ts) and now omits the new prompt.ts, so the enumeration no longer matches the module set. This is the exact line PR #301 (issue 291) updated when ste-style.ts was added, and the pipeline's docs-sync scope (pilot paths README.md, docs/**/*.md) does not cover extensions/grillme/README.md, so nothing downstream will fix it. Impact is documentation-only (tsc checks prompt.ts transitively via index.ts — verified), but the list is the module inventory. Suggested fix: --allowImportingTsExtensions index.ts bash-filter.ts prompt.ts ste-style.ts widget.ts.
extensions/grillme/prompt.ts:16 (with extensions/grillme/index.ts:18-22) — GrillPromptIntent/GrillPromptResearchMode/GrillPromptPhase/GrillPromptAlternative are declared a second time, copied from index.ts's Intent/ResearchMode/GrillPhase/GrillAlternative with comments that say only "Matches the … in index.ts" — two sources of truth for the same unions, with the prompt module unable to be the authority. Verified behaviour: tsc catches a divergence (Type 'Intent' is not assignable to type 'GrillPromptIntent') when Intent is widened, but node --test/type-stripping does not, and npm test is the only configured gate — so drift is silently loadable. Suggested fix: make prompt.ts authoritative and delete the local declarations in index.ts, e.g. import type { GrillPromptIntent as Intent, GrillPromptResearchMode as ResearchMode, GrillPromptPhase as GrillPhase, GrillPromptAlternative } from "./prompt.ts"; (type-only import, so index.ts still owns runtime behaviour), or move the shared unions into prompt.ts-exported aliases that index.ts re-uses for GrillState.
extensions/grillme/prompt.ts:114 — the extracted template is now testable but only two of its substitutions are asserted. outputPhaseGuidance()'s three branches are untested (the output branch carries the "ask the user instead of bypassing or faking success" rule plus GITHUB_REPO_PERMISSION_GUIDANCE, the output-selection branch carries the mandatory-phase rule), as are RESEARCH_GUIDANCE[input.researchMode] and the describeOutputPreference() fallback; the index.ts seam is untested too, because toPromptInput() (index.ts:78) hand-copies the 9-field mapping that interviewSample() mirrors by hand, so a swapped field would not fail any test. Suggested fix: add cases to prompt.test.ts for phase: "output" (assert GITHUB_REPO_PERMISSION_GUIDANCE and grill_finish_output_phase), phase: "output-selection", phase: "interview", each of the three researchMode strings, and an empty/whitespace outputPreference (assert the "(none set; …)" text); and move toPromptInput() into prompt.ts (e.g. toGrillPromptInput(state)) so the state→prompt mapping is covered rather than re-typed in the test fixture.

Low

extensions/grillme/prompt.test.ts:5 — the header comment justifies the extraction with "importing it would pull in pi and TypeBox, which node --test does not have", but both peers resolve in this checkout (node_modules/@earendil-works/pi-coding-agent, node_modules/typebox, also under extensions/grillme/node_modules — node -e "await import('./extensions/grillme/index.ts')" loads fine from the repo root), and node --test would too. The real, verified invariant is the dependency-free import chain (prompt.ts → ste-style.ts, so the suite runs with no node_modules — re-checked in a copy with none present). Suggested fix: reword to "the handler is not exported, and importing index.ts would couple the suite to pi and TypeBox; the builder is extracted so the prompt can be asserted without those imports" and keep the "runs with no node_modules" claim on the dependency-free chain alone.
AGENTS.md:26 — the branch commit is 6b081d5 293: Extract buildGrillPrompt() and add the prompt-integration test, but the repo convention for feature/bug/enhancement is issue-<N>: <summary> (issue-293: …); the merged squash commit and PR title from the previous two changes in this series are issue-292: … / issue-291: …. Suggested fix: use issue-293: <summary> for the commit/MR title at MR-creation time (Closes #293 in the body); no source change is needed.

Documentation

Updated files:

docs/GRILL_ME_STE_STYLE_PLAN.md

## 293: Extract buildGrillPrompt() and add the prompt-integration test Closes #293 — [293](https://git.excelera.net/david/pi-extensions-and-skills/issues/293). > 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 5m 57s. ## Review findings Now let me apply the fixes. Starting with the README typecheck line (Medium 1):Now the type-consolidation fix (Medium 2) and moving the state→input mapping into `prompt.ts` (Medium 3). First `prompt.ts`:Now `index.ts` — replace the duplicated type declarations with type-only imports and use the moved mapper:Now rewrite the test file to (a) reword the header comment and (b) cover the untested branches and the state→input seam:Now let's run the grillme tests and the documented typecheck:All 15 pass. Now the full project gate:Full suite green (1452 pass / 0 fail / 1 unrelated skip). Let me verify the module graph and review the full diff:All gates pass and the module graph loads cleanly. Here are the annotated findings. # Review Findings — 293 ## Summary Fixed all Medium findings and the Low documentation/comment finding. In `extensions/grillme/README.md`, the typecheck module inventory now includes `prompt.ts`. In `extensions/grillme/prompt.ts`, the duplicated `GrillPrompt*` unions are now the single source of truth — `index.ts` drops its local `Intent`/`ResearchMode`/`GrillPhase`/`GrillAlternative` declarations in favour of type-only aliased imports. The state→prompt mapper moved into `prompt.ts` as `toGrillPromptInput()` (called by `index.ts`), and `prompt.test.ts` now covers the phase branches (`interview`/`output-selection`/`output`), all three research modes, the output-preference fallback, and the state→input seam. The `prompt.test.ts` header comment was reworded to describe the real invariant. Gates: `node --test extensions/grillme/*.test.ts` passed (450 pass / 0 fail), `npm test` passed (1452 pass / 0 fail / 1 unrelated skip), and the documented `tsc --noEmit … index.ts bash-filter.ts prompt.ts ste-style.ts widget.ts` typecheck passed. Left unresolved: the `AGENTS.md:26` commit-message finding, which is an MR-creation-time action and cannot be fixed without committing/amending, which this stage forbids. ## Critical ## High ## Medium - [x] `extensions/grillme/README.md:90` — the documented typecheck line enumerates the extension's modules (`index.ts bash-filter.ts ste-style.ts widget.ts`) and now omits the new `prompt.ts`, so the enumeration no longer matches the module set. This is the exact line PR #301 (issue 291) updated when `ste-style.ts` was added, and the pipeline's docs-sync scope (`pilot` paths `README.md`, `docs/**/*.md`) does not cover `extensions/grillme/README.md`, so nothing downstream will fix it. Impact is documentation-only (tsc checks `prompt.ts` transitively via `index.ts` — verified), but the list is the module inventory. Suggested fix: ` --allowImportingTsExtensions index.ts bash-filter.ts prompt.ts ste-style.ts widget.ts`. - [x] `extensions/grillme/prompt.ts:16` (with `extensions/grillme/index.ts:18-22`) — `GrillPromptIntent`/`GrillPromptResearchMode`/`GrillPromptPhase`/`GrillPromptAlternative` are declared a second time, copied from `index.ts`'s `Intent`/`ResearchMode`/`GrillPhase`/`GrillAlternative` with comments that say only "Matches the … in `index.ts`" — two sources of truth for the same unions, with the prompt module unable to be the authority. Verified behaviour: `tsc` catches a divergence (`Type 'Intent' is not assignable to type 'GrillPromptIntent'`) when `Intent` is widened, but `node --test`/type-stripping does not, and `npm test` is the only configured gate — so drift is silently loadable. Suggested fix: make `prompt.ts` authoritative and delete the local declarations in `index.ts`, e.g. `import type { GrillPromptIntent as Intent, GrillPromptResearchMode as ResearchMode, GrillPromptPhase as GrillPhase, GrillPromptAlternative } from "./prompt.ts";` (type-only import, so `index.ts` still owns runtime behaviour), or move the shared unions into `prompt.ts`-exported aliases that `index.ts` re-uses for `GrillState`. - [x] `extensions/grillme/prompt.ts:114` — the extracted template is now testable but only two of its substitutions are asserted. `outputPhaseGuidance()`'s three branches are untested (the `output` branch carries the "ask the user instead of bypassing or faking success" rule plus `GITHUB_REPO_PERMISSION_GUIDANCE`, the `output-selection` branch carries the mandatory-phase rule), as are `RESEARCH_GUIDANCE[input.researchMode]` and the `describeOutputPreference()` fallback; the `index.ts` seam is untested too, because `toPromptInput()` (`index.ts:78`) hand-copies the 9-field mapping that `interviewSample()` mirrors by hand, so a swapped field would not fail any test. Suggested fix: add cases to `prompt.test.ts` for `phase: "output"` (assert `GITHUB_REPO_PERMISSION_GUIDANCE` and `grill_finish_output_phase`), `phase: "output-selection"`, `phase: "interview"`, each of the three `researchMode` strings, and an empty/whitespace `outputPreference` (assert the "(none set; …)" text); and move `toPromptInput()` into `prompt.ts` (e.g. `toGrillPromptInput(state)`) so the state→prompt mapping is covered rather than re-typed in the test fixture. ## Low - [x] `extensions/grillme/prompt.test.ts:5` — the header comment justifies the extraction with "importing it would pull in pi and TypeBox, which `node --test` does not have", but both peers resolve in this checkout (`node_modules/@earendil-works/pi-coding-agent`, `node_modules/typebox`, also under `extensions/grillme/node_modules` — `node -e "await import('./extensions/grillme/index.ts')"` loads fine from the repo root), and `node --test` would too. The real, verified invariant is the dependency-free import chain (`prompt.ts` → `ste-style.ts`, so the suite runs with no `node_modules` — re-checked in a copy with none present). Suggested fix: reword to "the handler is not exported, and importing `index.ts` would couple the suite to pi and TypeBox; the builder is extracted so the prompt can be asserted without those imports" and keep the "runs with no `node_modules`" claim on the dependency-free chain alone. - [ ] `AGENTS.md:26` — the branch commit is `6b081d5 293: Extract buildGrillPrompt() and add the prompt-integration test`, but the repo convention for `feature`/`bug`/`enhancement` is `issue-<N>: <summary>` (`issue-293: …`); the merged squash commit and PR title from the previous two changes in this series are `issue-292: …` / `issue-291: …`. Suggested fix: use `issue-293: <summary>` for the commit/MR title at MR-creation time (`Closes #293` in the body); no source change is needed. ## Documentation Updated files: - docs/GRILL_ME_STE_STYLE_PLAN.md

david added 3 commits

2026-10-09 21:50:08 +00:00