# LLM-Based Quality Gate — Checklist

Each `- [ ] <question>` line below becomes one independent Gemini call on every PR. The workflow parses this file at runtime; you can add, edit, or remove items here without touching workflow YAML.

Author guidance:
- Phrase items as **questions a careful reviewer would ask**, not abstract category labels.
- One question per line. The whole question must be on the same line.
- Reference concrete paths in the question when you can — the model uses them as search hints.
- Keep the list focused. The whole point of one-question-per-call is that each call stays simple and the model can focus.

## Active items

- [ ] **Code reuse**: Does this PR introduce a new helper, parser, formatter, or utility that re-implements logic already present in `src/utils/`, `src/core/`, `src/commands/`, or any `packages/*/src/`? Cite both the new code and the existing helper.
- [ ] **Template / canonical-source semantic fidelity**: If this PR modifies a `content/templates/*/template.md`, `content/templates/*/template.json`, or `content/templates/*/metadata.yaml`, are user-facing field semantics preserved? Specifically: are any required fields silently dropped or renamed, are placeholder tags removed without a corresponding metadata change, and does the change risk breaking downstream DOCX rendering or recipe playback in ways `validate.yml` would not catch?
- [ ] **Cross-file consistency on removals**: If this PR deletes a feature, option, function, or script (or a CLI flag, or a config key), are all references cleanly removed across the codebase? Look in `src/`, `packages/*/src/`, `api/`, `scripts/`, `integration-tests/`, and `docs/`. This catches the `templatesOnly` class of bug where the same removal had to be re-applied across PRs.
- [ ] **MCP contract consistency**: If this PR adds or modifies an MCP tool in `src/`, `packages/contracts-workspace-mcp/`, `packages/contract-templates-mcp/`, `packages/checklist-mcp/`, or `api/_*`, does the tool's input/output schema match the documented envelope shape (see `packages/contracts-workspace/` types and the shared envelope used by `signing/`)? Are errors normalized through the shared envelope rather than thrown raw? Are tool names, versions, and capabilities in sync between `gemini-extension.json`, `mcp.json`, the implementation file, and any tests under `integration-tests/`?
- [ ] **Registries and hard-coded identifiers (DRY)**: This item targets **declarative data and identity sets** (literal lists, allowlists, ID predicates, projections), distinct from Item 1 which targets imperative helper logic. If this PR adds a new section type, allowlist value, OA-owned-template predicate, OpenSpec scenario ID, or shared field projection, does it update an existing data registry rather than introducing another scattered literal list? Check `scripts/template_renderer/canonical-source.mjs`, `scripts/template_renderer/schema.mjs`, `scripts/template_renderer/layouts/*`, `scripts/lib/template-utils.mjs`, `openspec/`, and `integration-tests/`. If multiple lists remain after this PR, cite each and justify why consolidation isn't feasible here.
- [ ] **Fill-pipeline string-transform safety**: If this PR adds or modifies a string transform inside `src/core/`, `src/commands/fill.ts`, or `packages/contracts-workspace/` that operates on paragraph or run text, does it (a) preserve `w:fldChar` field-code runs so PAGE/NUMPAGES survive (#185), (b) preserve trailing whitespace that callers may rely on as anchors (#109), (c) treat smart vs straight punctuation consistently with how templates are authored (#86), and (d) avoid truthiness checks on placeholder strings that may be empty but non-null (#148)? Cite the transform and the case it could miss.
- [ ] **Pages.app DOCX compatibility**: If this PR modifies a `content/templates/*/template.docx`, its style tree (`styles.xml`, `pPrDefault`, `Normal`), or any DOCX-generating code under `scripts/template_renderer/layouts/*` / `scripts/generate_*template*.mjs` / `src/core/checklist/*`, does every paragraph reference a named style (`OATitle`, `OAClauseHeading`, `OABlockSignatureFollow`, or `Normal`) rather than inline alignment without `pStyle`? Are paragraph-spacing defaults non-zero so Pages renders vertical whitespace? Are alignment, spacing, emphasis, and indentation expressed via real OOXML rather than NBSP/zero-spacing hacks? (See #262, #263, #74.)
- [ ] **Catalog and projection fan-out**: If this PR (a) adds a new template directory under `content/templates/`, (b) modifies the `fields:` block in any `content/templates/*/metadata.yaml`, or (c) changes a field projection in `packages/contracts-workspace/`, does the change flow through the shared projection and every checked-in consumer artifact? Check `src/core/template-listing.ts`, `api/_shared.ts`, `api/mcp.ts`, `api/a2a.ts`, `packages/contract-templates-mcp/`, and `data/templates-snapshot.json`. Verify any external catalog or snapshot that isn't auto-derived by CI is refreshed in the same PR (#264 → #275). Flag any "full" list shape that duplicates a `get_*` response (#267 → #274). **Clause-text-only edits to `template.md` or to the `clauses` arrays in `template.json` do NOT trigger this item** — `data/templates-snapshot.json` only tracks fields and metadata, not clause content or document version, so clause-only edits never alter snapshot bytes (verified: PR #326 added a clause without changing snapshot bytes; `node scripts/export-templates-snapshot.mjs --check` confirms).
- [ ] **Signing surface — anchors, env vars, auth threading**: If this PR touches `packages/signing/`, `api/signing/`, `content/templates/*/signing.yaml`, or any `content/templates/*/` file referenced by a signing recipe, verify (a) anchor strings declared in the template's `signing.yaml` are still injectable into the rendered DOCX (#146, #109), (b) any new `process.env.DOCUSIGN_*` reads are trimmed before use (#133, #134), (c) any new signing tool is gated on DocuSign config presence rather than crashing when unconfigured (#213), and (d) auth fields like `__auth_sub` are threaded into `params.arguments` for any tool that needs them (#160).
- [ ] **Selector paragraph removal — bookmark balance**: If this PR adds a new group-processor or paragraph-removal path in the selector engine (`src/core/selector.ts`), does it route through `removeWithBookmarkCleanup()` (or equivalent) so `bookmarkStart`/`bookmarkEnd` pairs stay balanced after removal? Reference: #120.
- [ ] **Publish-artifact completeness and boundary self-containment**: If this PR adds or renames a file referenced from `gemini-extension.json`, `mcp.json`, `server.json`, or any `package.json` manifest, is the file included in the corresponding `files` array (or covered by a wildcard) and exercised by `scripts/check_*` or a tarball-content assertion in `.github/workflows/release.yml` (#151, #50, #87)? If a package or skill published from a subdirectory now depends on symlinks, `../shared/*`, or files outside its publish boundary, will the target publisher actually include them (#161, #162)? Flag any new test that pulls a broad module like `api/_shared.ts` where a narrower helper would avoid unrelated dependency drag (#182, #187).
- [ ] **OpenSpec / contract-test scenario ID uniqueness**: If this PR adds a new `OA-DST-*` or `OA-SIG-*` scenario ID under `openspec/`, `integration-tests/`, or `packages/*/tests/`, is the ID free across (a) merged scenarios in `openspec/specs/` and `integration-tests/`, and (b) any in-flight `openspec/changes/*/` that reserves a range? `openspec validate --strict` does not detect cross-corpus collisions (#293). Cite the chosen ID and the highest currently-claimed ID in the same prefix family.
- [ ] **Parser edge-case coverage**: If this PR modifies markdown/frontmatter/table parsing in `scripts/template_renderer/canonical-*.mjs` (or any new canonical-source parser), do the tests under `integration-tests/canonical-*` cover CRLF, escaped delimiters such as `\|`, missing delimiters, and precedence/error branches — not just happy paths? If the parser can now choose between two anchor mechanisms or throw on malformed structure, point to the exact regression test that proves the intended branch fires (#270, #286, #291, #306).
- [ ] **Load-bearing tests**: If this PR adds or modifies tests for auth-gated tools, parser edge cases, envelope/wrapper behavior, or error-path handling, do the tests unconditionally exercise the intended production path rather than short-circuiting before it? Review `integration-tests/`, `api/mcp.ts`, `packages/signing/src/*`, and any mocks for auth/module loading. Flag conditional assertions like `if (statusCode === 200) { ... }` or branch coverage that never proves the critical path ran (#236 → #293, #286, #291).
