Skip to content

memorize: always verify math actually renders - #155

Merged
d-morrison merged 3 commits into
mainfrom
claude/fervent-driscoll-ee0b9e
Jun 22, 2026
Merged

memorize: always verify math actually renders#155
d-morrison merged 3 commits into
mainfrom
claude/fervent-driscoll-ee0b9e

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Captures a standing preference: when adding or changing math (LaTeX/Quarto
equations), verify it actually renders — open the rendered HTML page and
confirm the equation displays, not just that the build succeeded. A typo in a
macro can silently break MathJax while the build still passes.

Added one bullet to memories/preferences.md, next to the existing
inline-R-expressions rule, framed as an instance of never assume; always
verify
applied to math. Example given is the rme PR preview.

Came from a direct user directive ("always check that math you add or change
actually renders"), recorded via the memorize skill.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 22, 2026 19:04
@github-actions
github-actions Bot removed the request for review from Copilot June 22, 2026 19:04
@claude

This comment has been minimized.

Comment thread memories/preferences.md Outdated
Comment thread memories/preferences.md Outdated
@d-morrison

Copy link
Copy Markdown
Collaborator Author

Driving this PR to clean — back off until done.

Addresses PR #155 review: the hardcoded pr-772 preview link is ephemeral
(deleted when that PR closes) — the very anti-pattern the rule warns against;
use a pr-<N> placeholder. And match 'never assume; always verify' wording.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 22, 2026 19:48
@github-actions
github-actions Bot removed the request for review from Copilot June 22, 2026 19:48
@claude

This comment has been minimized.

Copilot AI review requested due to automatic review settings June 22, 2026 19:48
@d-morrison

Copy link
Copy Markdown
Collaborator Author

ARD summary — round 1

# Finding Disposition What changed
1 preferences.md — hardcoded pr-772 preview URL is ephemeral (deleted when that PR closes) — the very anti-pattern the rule warns against Addressed Replaced with a pr-<N> placeholder and a note that per-PR previews are deleted on close
2 preferences.mdnever-assume hyphenated, inconsistent with the rest of the file Addressed Changed to never assume; always verify

Also merged current main into the branch (it had moved ahead with #154 and #156) so the review runs against an up-to-date branch. Pushed in 51dc99d (fixes) + merge 0402870.

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.

Pull request overview

Adds a new standing preference to the repository’s recorded guidelines: when editing LaTeX/Quarto math, verify the rendered output (HTML/MathJax) visually rather than relying solely on a successful build.

Changes:

  • Documented a “verify math renders” rule with examples of common math syntaxes.
  • Added a concrete verification workflow example using the rme PR preview HTML page pattern.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@claude

This comment has been minimized.

@d-morrison

Copy link
Copy Markdown
Collaborator Author

ARD summary — round 2

Both round-1 findings confirmed fixed. One non-blocking observation remained:

# Observation Disposition Rationale
1 Example URL still has a concrete chapter slug (chapters/proportional-hazards-models.html) Rebut / keep That's the actual page the rule's author pointed at, and the slug is stable. Keeping it concrete makes the example more useful than an abstract <chapter> placeholder; the PR number — the genuinely ephemeral part — is already a <N> placeholder. Reviewer agreed it's fine as-is.

No code changed this round, so re-requesting a confirming review.

@claude review

@github-actions

Copy link
Copy Markdown
Contributor

👀 Picked up by workflow run #27979718767. Setup runs first; Claude itself responds after that.

@d-morrison

Copy link
Copy Markdown
Collaborator Author

Done — PR is clean (CI green, no review findings, all inline threads resolved). PR is free.

@d-morrison
d-morrison merged commit 926688c into main Jun 22, 2026
4 checks passed
@d-morrison
d-morrison deleted the claude/fervent-driscoll-ee0b9e branch June 22, 2026 20:27
d-morrison added a commit that referenced this pull request Jun 22, 2026
…URLs (#159)

UMS follow-up from PR #155's review lifecycle.

- skills/memorize/SKILL.md: document the worktree / occupied-main-checkout
  case. ~/.claude/memories and ~/.claude/CLAUDE.md symlink to the MAIN checkout,
  so rev-parse --show-toplevel resolves there, not your worktree; if that
  checkout is on another branch, edit the memory in your worktree and land it
  via branch+PR+merge.
- memories/preferences.md: extend the "don't bake a volatile value into prose"
  rule to cover ephemeral example URLs (PR-preview links, PR numbers) —
  parameterize the ephemeral part (pr-<N>).
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.

2 participants