docs(templates): clarify /constitution's Sync Impact Report is temporary, review-only material (#4431)

[chelsealong]

Fixes #4431

Problem

templates/commands/constitution.md (Step 4 of the Outline section) instructed
agents to "prepend" the Sync Impact Report as an HTML comment at the top of
.specify/memory/constitution.md, with no instruction to remove a previous
report first. Because HTML comments are invisible in rendered Markdown but
fully present in the raw file, every /constitution run stacked another
<!-- ... --> report block on top of the last one. Any agent or extension
hook that loads the raw constitution file at runtime (/specify, /review,
before_constitution/after_constitution hooks, etc.) pays a growing,
unbounded token cost reading historical changelog data that has no
governance value at inference time.

Fix

Step 4 now explicitly instructs the agent to remove any existing HTML
comment at the top of the file before adding the new Sync Impact Report,
so the file never carries more than one report — it is replaced, not
stacked.

This is a one-line-of-behavior instruction change to the command template
(templates/commands/constitution.md); no other copies of this text exist
in the repo.

Test plan

Added tests/test_constitution_template_sync_report.py, which asserts the
Step 4 instructions require removing any prior Sync Impact Report comment
rather than only prepending.

• Confirmed the test fails against the pre-fix template
  (git checkout HEAD~1 -- templates/commands/constitution.md):

  tests/test_constitution_template_sync_report.py::test_sync_impact_report_step_instructs_removing_prior_report FAILED
  AssertionError: assert ('never stack' in '...' or 'not stack' in '...')
  

• Restored the fix and confirmed the test passes:

  tests/test_constitution_template_sync_report.py::test_sync_impact_report_step_instructs_removing_prior_report PASSED
  1 passed in 0.18s
  

• Ran the full suite: uv run --extra test pytest -q

  7705 passed, 12 skipped, 48 warnings in 499.34s (0:08:19)
  

AI disclosure

This change, including the code, tests, and this PR description, was
generated autonomously by an AI coding agent (Claude, model: claude-sonnet-5)
acting on behalf of the repository owner, with no line-by-line human review
prior to commit.

Assisted-by: Claude (model: claude-sonnet-5, autonomous)

[mnriem]

As indicated by the comment on the issue please change this to a documentation issue

[chelsealong]

Agreed, retitled as a docs clarification. Pushed 609dc58: Step 4 now states explicitly that the Sync Impact Report is temporary, review-only material expected to be removed before the constitution file is committed, alongside the existing replace-not-stack instruction for runs where a report was left in place (templates/commands/constitution.md). Test suite still green (tests/test_constitution_template_sync_report.py).

[Copilot]

🟡 Changes recommended

The removal instruction can delete unrelated leading HTML comments.

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

Pull request overview

Clarifies that /constitution Sync Impact Reports are temporary and must be replaced rather than stacked.

Changes:

• Adds replacement guidance to the constitution command template.
• Adds a regression test for the guidance.

File summaries

File	Description
templates/commands/constitution.md	Documents report replacement and removal.
tests/test_constitution_template_sync_report.py	Verifies prior-report removal guidance.

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.

[mnriem]

Thanks for adding the lifecycle wording — that part is exactly right. But I'd like this to be documentation-only, and right now it still carries the behavioral change I was pushing back on.

Two requests:

1. Drop the de-dupe instruction. Please remove the bullet that tells the agent to strip a pre-existing HTML comment:

"If the file already starts with an HTML comment … remove it entirely before adding the new one. The file must never carry more than one Sync Impact Report; replace, never stack."

In the intended workflow the Sync Impact Report is temporary scratch material that's removed before the amended constitution is committed, so /constitution shouldn't be auto-stripping prior reports — that's a behavior change to work around a state that shouldn't occur. Keep just the lifecycle sentence you added.

2. Retarget the test. test_constitution_template_sync_report.py currently asserts the removal/never-stack behavior ("remove", "never stack"). Please change it to assert the documented lifecycle instead — e.g. that Step 4 states the report is temporary/review-only and is expected to be removed before commit. The test should pin the documentation, not the de-dupe logic we're removing.

Net: Step 4 gains only the clarification that the report is temporary and removed before commit; no new agent behavior. Re-request review once that's in.

[chelsealong]

Pushed e3870d6: removed the de-dupe bullet from Step 4 (no more auto-stripping a leading HTML comment), keeping only the temporary/removed-before-commit lifecycle wording. Retargeted tests/test_constitution_template_sync_report.py to assert that documented lifecycle instead of the removal/never-stack behavior. Full suite green (7705 passed, 12 skipped).