Skip to content

memorize: stacked-PR doc-ancestry rule (general) + sparta website hook - #162

Merged
d-morrison merged 4 commits into
mainfrom
memory/sparta-website-docs-scope
Jun 23, 2026
Merged

memorize: stacked-PR doc-ancestry rule (general) + sparta website hook#162
d-morrison merged 4 commits into
mainfrom
memory/sparta-website-docs-scope

Conversation

@d-morrison

@d-morrison d-morrison commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Captures the lesson from sparta PR #185, where website docs were written for a feature (order_response_delay) that lived in a sibling branch, not in the PR's own ancestry — the reviewer flagged it as a hallucinated feature.

Per review feedback, split into two scopes instead of a single sparta-only note:

  • General rulememories/preferences.md: "only document features present on the current branch's ancestry — grep first." Fires in any repo, framed as a specific case of the standing "NEVER assume; ALWAYS verify" rule.
  • Sparta hookmemories/repo/sparta.md: the sparta-specific website-update policy that makes this trap likely, the concrete ums: gha#92 post-merge learnings #185 example, and a pointer to the general rule.

Local validators pass (validate-skills.py, check-vendored-drift.py).

Copilot AI review requested due to automatic review settings June 22, 2026 21:05
@github-actions
github-actions Bot removed the request for review from Copilot June 22, 2026 21:05
@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 2m 56s —— View job


Code Review — PR #162

  • Gather context and read changed files
  • Review the memory file content
  • Check cross-file consistency
  • Post findings

Verdict: Approve with one cross-file fix needed

The content is correct and well-structured. The rule is clear, the motivation is concrete (referencing PR #185 specifically), and the Why/How-to-apply format matches the memory spec. No AI tells, no hallucinated references.

One finding — not in the changed file, but triggered by it:

memories/tools.md lines 253–255 are now factually wrong

The tools.md memory says:

Memory files (memories/*.md) have no YAML frontmatter — the file starts directly with a # heading. grep -r "^name:" memories/ always returns empty; don't use frontmatter fields to locate or identify a memory file.

Both memories/repo/rme.md (already merged) and memories/repo/sparta.md (this PR) use YAML frontmatter with name:, description:, and metadata.type: fields. The grep claim is no longer true.

A future session reading tools.md may:

  • strip frontmatter it finds in memory files (treating it as a mistake)
  • grep for ^name: in memories and draw a wrong conclusion when it finds nothing

The tools.md section should be updated to reflect current practice — either delete the no-frontmatter claim or rewrite it to match how rme.md and sparta.md are actually structured.

This can be fixed in this PR or a follow-up. Since it's cross-file and already an existing gap (rme.md predates this PR), a follow-up is fine — but a tracking issue would keep it from being lost.


The "only document features on the current branch's ancestry" lesson is a
general stacked-PR working rule, not sparta-only. Promote it to a cross-workspace
bullet in preferences.md (a specific case of "NEVER assume; ALWAYS verify") so it
fires in any repo. memories/repo/sparta.md keeps just the sparta-specific hook —
the website-update policy that makes the trap likely — plus the concrete #185
example and a pointer to the general rule.
Copilot AI review requested due to automatic review settings June 23, 2026 00:46
@github-actions
github-actions Bot removed the request for review from Copilot June 23, 2026 00:47
@d-morrison d-morrison changed the title memorize: sparta website docs must match branch ancestry memorize: stacked-PR doc-ancestry rule (general) + sparta website hook Jun 23, 2026
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 4m 0s —— View job


Code Review — PR #162

  • Gather context
  • Read changed files
  • Check what changed since prior review
  • Post findings

Confirming review — prior verdict stands, new commit is clean

The prior review (2026-06-22) approved the original sparta.md with one cross-file finding: the tools.md claim that memory files have no YAML frontmatter (memories/tools.md:276–278) is stale.

The new commit (0ed747d) since that review — split doc-ancestry rule into general preference + sparta hook — is clean and is an improvement on the original. It:

  • Adds a portable general rule to memories/preferences.md (lines 105–110): only document features present in the current branch's ancestry; grep for the symbol first; move docs to the branch where the code lives.
  • Trims memories/repo/sparta.md to cross-reference that rule instead of restating it, keeping only the sparta-specific context (the website/ policy, the concrete PR ums: gha#92 post-merge learnings #185 case).

The split is the right structure. No new issues in either changed file. No AI tells, no line-number citations, no hallucinated references in the new content.

Still open from the prior review

memories/tools.md:276–278 still claims memory files have no YAML frontmatter and that grep -r "^name:" memories/ returns empty. Both are now false (sparta.md and rme.md both use frontmatter). A future session reading that section may strip frontmatter it finds, or draw wrong conclusions from the grep.

The prior review said a follow-up issue is fine since the gap predates this PR. If one hasn't been filed yet, please track it — that's the only open item blocking a fully-clean verdict.


memories/*.md may carry YAML frontmatter (name/description/metadata) — e.g.
memories/repo/sparta.md, added in this PR — so the old "no frontmatter, grep
returns empty" claim is false and could lead a future session to strip valid
frontmatter. Rewrite to say frontmatter is optional and must be preserved.

Closes the only open item from the PR #162 review.
Copilot AI review requested due to automatic review settings June 23, 2026 00:52
@d-morrison

Copy link
Copy Markdown
Collaborator Author

Addressed the one open item. The stale tools.md:276–278 claim that memory files have no YAML frontmatter is now false — this PR itself adds the frontmatter'd memories/repo/sparta.md — so rather than defer, I fixed it in 01e9ce2: the section now says frontmatter is optional (name/description/metadata), shows how to grep for it, and warns to preserve rather than strip it. No follow-up issue needed.

Validators still pass. The push re-triggers the review automatically.

@github-actions
github-actions Bot removed the request for review from Copilot June 23, 2026 00:52
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 1m 10s —— View job


Code Review — PR #162

  • Gather context
  • Read changed files
  • Verify the tools.md fix addresses the prior finding
  • Post findings

Confirming review — PR is fully clean

The second prior review (2026-06-23) approved the content changes (preferences.md, sparta.md) and left one open item: memories/tools.md still falsely claimed memory files have no YAML frontmatter. That item is now closed.

Commit 01e9ce2 rewrites memories/tools.md:275–280 correctly:

  • Before: asserted memory files have no YAML frontmatter and that grep -r "^name:" memories/ always returns empty.
  • After: says frontmatter is optional (may carry name/description/metadata), gives the correct grep to find frontmatter'd files, and explicitly warns to preserve rather than strip existing frontmatter.

The fix is accurate, complete, and directly counters both failure modes the prior review identified (a session stripping frontmatter it finds, or drawing wrong conclusions from a grep). The example file cited (memories/repo/sparta.md) is the file this PR adds — no stale reference. No AI tells, no hallucinated symbols.

All three changed files are clean. No new findings.

@d-morrison

Copy link
Copy Markdown
Collaborator Author

ARDI complete — the latest review confirms all three changed files clean with no findings, and all CI is green. PR is ready to merge.

@d-morrison
d-morrison merged commit 79346e5 into main Jun 23, 2026
4 checks passed
@d-morrison
d-morrison deleted the memory/sparta-website-docs-scope branch June 23, 2026 02:22
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.

1 participant