Skip to content

fix(implement): count checkbox markers outside code fences only (#4272) - #4313

Open
ntdatt812 wants to merge 1 commit into
github:mainfrom
ntdatt812:fix/4272-checklist-code-fences
Open

fix(implement): count checkbox markers outside code fences only (#4272)#4313
ntdatt812 wants to merge 1 commit into
github:mainfrom
ntdatt812:fix/4272-checklist-code-fences

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

Closes #4272.

templates/commands/implement.md told the agent to count checkbox markers like this:

- Total items: All lines matching `- [ ]` or `- [X]` or `- [x]`

Every line, fenced blocks included. So a checklist that documents the checkbox format — an example fence showing what - [ ] looks like — reported unchecked items that no reviewer can ever tick, and the gate stops on a non-zero unchecked count. Writing down the format blocked implementation.

templates/commands/clarify.md already had the right rule and the right words for it:

lines matching - [ ], - [x], or - [X]outside of code fences

So this was also two commands disagreeing about what a checklist item is, which is the part worth fixing rather than just the count.

Only templates/commands/implement.md needed the change: grep for the scan instruction across templates/commands/ and presets/*/commands/ returns exactly these two files, and the lean preset's speckit.implement.md does not carry a counting block.

Test

tests/unit/test_checklist_scan_contract.py turns the rule into a contract rather than a one-time edit. It walks every command template, finds each line that defines what counts as a checkbox marker, and asserts that line also excludes code fences. It is parametrized per instruction, so the failure names the file, the line number and the offending text:

FAILED …::test_marker_scans_exclude_code_fences[implement.md-63-     - Total items: All lines matching `- [ ]` …]

Two details that make it a real guard rather than a passing test:

  • a companion test_the_contract_is_actually_stated_somewhere fails if the regex ever matches nothing, so deleting or rewording the scan instruction cannot make the suite green by making the parametrization empty;
  • it scans presets/*/commands too, so a preset that grows its own counting block is held to the same rule.

Mutation-checked — restoring the old wording in implement.md fails exactly one case, the implement one, and the other three stay green:

1 failed, 3 passed
python -m pytest tests/unit/test_checklist_scan_contract.py -q
4 passed

No behaviour outside the checklist scan changes: the per-checklist total/checked/unchecked reporting and the status table are untouched.

The checklist gate counted every `- [ ]` / `- [x]` line in every checklist
file, fenced blocks included. A checklist that documents the checkbox format
with an example fence therefore reported unchecked items nobody can ever tick,
and /speckit-implement stops on a non-zero unchecked count -- so writing down
the format blocked implementation.

/speckit-clarify already scopes its scan to markers outside code fences, so
this was also the two commands disagreeing about what a checklist item is.
They now state the same rule.

Closes github#4272

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The contract regex misses checked-only scan instructions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Aligns implementation checklist scanning with clarify by excluding fenced examples.

Changes:

  • Clarifies checklist counting rules.
  • Adds a cross-template contract test.
File summaries
File Description
templates/commands/implement.md Excludes fenced checkbox examples.
tests/unit/test_checklist_scan_contract.py Enforces scan wording across templates.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


# The instruction that tells the agent which lines are checkbox markers. Written to catch
# the phrasing both commands use rather than one exact sentence.
SCAN_INSTRUCTION = re.compile(r"lines matching\s+`- \[ \]`", re.IGNORECASE)

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address Copilot feedback. If not applicable, please explain why

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: /speckit-implement counts checkbox markers inside fenced code blocks — example checkboxes can falsely block implementation

3 participants