Capture Learnings 286 287

[FrankRay78]

Why

Two learnings from building #286 and #287, approved at a /capture-learnings checkpoint.

What changes

• /build stops when local main has unpushed commits. It branches from origin/main, so an unpushed commit silently drops out of the new branch. This happened with Make a /speckit.reviewissue comment triageable, and stop the category sweep inflating it #281's study record: committed to local main but never pushed, it was missing from Escaping rule for a literal pipe in an at-a-glance table cell #286's branch even though Escaping rule for a literal pipe in an at-a-glance table cell #286 cited it as evidence.
• The grep-after-simplifying memory now covers adding a rule. A new rule in a command prompt must be reconciled with the older rules it overrides and scoped for every path it applies to (first run, re-run, automated workflow). speckit.reviewissue.md missed this on two issues in a row: Escaping rule for a literal pipe in an at-a-glance table cell #286's self-check clause and Check that a /speckit.reviewissue recommendation's Reason holds up against what it cites #287's Reason rule both clashed with the re-run rules in step 6, and /verify reviewers caught both.

Non-obvious things a reviewer should know

• /study still doesn't push. Every pipeline command only commits and /raise-pr alone pushes, so the stranded-record problem is fixed at /build's entry instead.
• The memory was updated in place, not duplicated. Its title and index line changed to match.

Related

• Escaping rule for a literal pipe in an at-a-glance table cell #286 / Table Pipe Escaping #288, Check that a /speckit.reviewissue recommendation's Reason holds up against what it cites #287 / Review Reason Fact Check #289; findings recorded in docs/study/286.md and docs/study/287.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_016g3xHRn9ChW4mcjvduzB2U

[FrankRay78]

@claude Review this pull request. Analyse the code changes and provide feedback covering:

• Bugs or correctness issues
• Security concerns (including any flagged by static analysis)
• Adherence to the project conventions in CLAUDE.md
• Test coverage — do the tests adequately cover the new behaviour?
• Any spec/test-plan mismatches

End the review with a recommendation whether to merge the PR, and if not, what you suggest needs addressing first.

[claude]

Claude finished @FrankRay78's task in 2m 39s —— View job

---

Review

• Gather context (PR diff, changed files)
• Read affected files in full (build.md, memory files, referenced study docs)
• Check correctness of the new /build guard
• Check the memory-file edits for internal consistency and cross-references
• Check CLAUDE.md/constitution conventions adherence
• Assess test coverage expectations for this kind of change
• Post final review with merge recommendation

Bugs / correctness

build.md:42 — git log origin/main..main --oneline is the correct direction to detect unpushed local commits (commits reachable from main but not origin/main), and it's placed after the git fetch origin main bullet so origin/main is fresh before the comparison. No issue with the check itself.

One real finding, and it's a live instance of the exact bug class this PR is fixing (medium confidence — worth confirming before merge):

The new feedback_grep_after_simplifying.md rationale and the PR body both cite docs/study/287.md as evidence. That file does not exist on this branch or on origin/main (tip is still 50df3a1) — it only exists on the unmerged remote branch origin/feature/287-review-reason-fact-check (commit 3ee3107, itself sitting on top of 50df3a1). In other words, PR #289 (the fix for #287) hasn't merged yet. If #290 merges first, feedback_grep_after_simplifying.md will cite a doc file that doesn't exist anywhere in main — the same "stranded evidence" shape /build's new guard exists to catch, just at the PR-merge-order level instead of the branch-push level. Recommend merging #289 before (or together with) this one, or dropping the docs/study/287.md citation until it lands.

Security

None — this PR only edits Markdown prompt/memory files, no executable code or CI paths touched.

CLAUDE.md / constitution adherence

• Markdown is soft-wrapped (one line per paragraph/bullet), consistent with feedback_markdown_soft_wrap.
• The rule is added locally in build.md rather than via a cross-referenced canonical section, consistent with feedback_prompts_locality_over_dry.
• Commit message uses #286 and #287 with no GitHub closing keyword, correct per the constitution's git-workflow rule.
• MEMORY.md's index line for the renamed memory was updated in place (title/description now match the file's own name:/description: frontmatter) rather than duplicated — matches what the PR body claims.
• Minor nit: the updated MEMORY.md line (.claude/memory/MEMORY.md:10) is ~200 characters, longer than the ~150-char guideline for index-line brevity (already true of the line it replaced at ~175 chars, so this pre-dates the PR — not a regression, just not improved either).

Test coverage

No tests are expected or missing here. Per the TDD carve-out in CLAUDE.md/constitution §I, these are prompt/tooling changes, not production code, and there's no automated tool that can execute a Claude Code slash-command prompt to gate it (unlike, say, dotnet format --verify-no-changes). This matches how prior prompt-only PRs in this repo (e.g. #288) were evidenced — via the docs/study/*.md finding/fix table rather than xUnit tests.

Spec/test-plan mismatches

N/A — no spec.md/test-plan.md involved; this is a /capture-learnings memory-update PR outside the spec-kit pipeline.

Recommendation

Hold merge on the docs/study/287.md reference, otherwise good to merge. The /build guard and the widened memory rule are both correct, well-motivated, and internally consistent (the new [[feedback_audit_class_after_two_failures]] link is apt). The only thing to resolve first: confirm PR #289 lands before/with this one so the new memory file's citation resolves to a real file on main, rather than merging a rule about stranded evidence that is itself, briefly, stranded evidence.