Skip to content

Teaching: a long remark is speaker notes on slides and a box on the web page - #124

Merged
d-morrison merged 5 commits into
mainfrom
claude/project-thread-hzfopq
Oct 8, 2026
Merged

d-morrison merged 5 commits into
mainfrom
claude/project-thread-hzfopq

Conversation

@d-morrison

@d-morrison d-morrison commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Requested by Ezra · project thread

Before: The teaching chapter said nothing about where commentary and long paragraphs go, so slides could carry text the audience is not meant to read.

After: The chapter says to put on a slide only what the audience should look at. Examples and short commentary go in #exm- and #rem- divs, not plain ::: notes. A long commentary paragraph goes in ::: {#rem-name .remark .notes}, which is speaker notes on the slides and a boxed remark on the web page, where attendees who missed the lecture read it. A separate short #rem- box is added only when the slide needs a takeaway.

How: two bullets under "Structure slides for the reader" in chapters/teaching.qmd. The agent rule is merged in ai-config#4372. The .remark class is kept to match that rule and quarto-remarks-vs-callouts; one reviewer round called it redundant with the #rem- prefix, which is harmless.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FMgJ6WAaWxMbtT7f8pst2K

claude added 3 commits October 8, 2026 03:26
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMgJ6WAaWxMbtT7f8pst2K
…eb page

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMgJ6WAaWxMbtT7f8pst2K
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@d-morrison d-morrison changed the title Teaching: long remarks get a short box and speaker notes Teaching: a long remark is speaker notes on slides and a box on the web page Oct 8, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMgJ6WAaWxMbtT7f8pst2K
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

Claude finished review — View run


Review: PR #124 — commit range 9319b5e1..fcc47da2

Change: chapters/teaching.qmd:163 — commit ab64dbb ("Teaching: keep the .remark class that draws the remark box") reverts the previous round's cleanup, changing the example back from ::: {#rem-name .notes} to ::: {#rem-name .remark .notes}. No other lines changed; the merge commit fcc47da carries no additional diff.

Re-verification against the actual rendering behavior

The commit message asserts that the explicit .remark class is what "draws the remark box," implying the prior round's render-based finding (that #rem- alone is sufficient) was wrong. I checked this against the real quarto-cli source rather than trusting either claim at face value:

  • src/resources/filters/customnodes/proof.lua's is_proof_div() first resolves the ID prefix via refType(), then looks it up in crossref.categories.by_ref_type.
  • src/resources/filters/mainstateinit.lua registers rem as a built‑in crossref category with name = "Remark" (alongside fig, tbl, prf, sol, etc.) at module init (setup_crossref_category_indices()).
  • Because "rem" is a registered ref_type, is_proof_div() finds crossref.categories.by_ref_type["rem"], takes tbl.name:lower() → "remark", and returns proof_types["remark"] (truthy) — so the #rem- ID prefix alone is sufficient to classify the div as a Remark/Proof node and render the boxed "Remark" callout. The explicit .remark class only matters as a fallback (proof_type(div) checking div.attr.classes) for divs that have no #rem-‑prefixed ID at all.

This independently confirms the previous review round's render test was correct, and the commit's stated rationale ("keep the .remark class that draws the remark box") is factually incorrect — the box is drawn by the #rem- ID prefix regardless of whether .remark is also present. Adding the class back is harmless at render time (Quarto's class handling tolerates the duplicate remark class with no functional change, confirmed by the earlier round's actual render of {#rem-foo .remark .notes}), but it:

  • contradicts this same book's own canonical syntax doc, chapters/defining-terms/using-theorem-environments.qmd:18 ("#rem- for remarks"), which lists no accompanying .remark class anywhere, and
  • re-introduces exactly the inconsistency the first review round flagged and the second round confirmed was fixed.

Findings (non-blocking)

  1. chapters/teaching.qmd:163 — Reverting to ::: {#rem-name .remark .notes} undoes the previously-applied, previously-confirmed fix, and the commit message's justification for doing so doesn't hold up against the actual quarto-cli source (see above): the #rem- prefix alone already produces the Remark box, so .remark remains redundant. Not a functional bug (nothing breaks), but worth reverting back to ::: {#rem-name .notes} for consistency with chapters/defining-terms/using-theorem-environments.qmd, unless the author has evidence of a real case (e.g., a specific Quarto/extension version or config) where #rem- alone fails to apply the box, in which case that evidence should be stated in the PR rather than asserted in a commit message.

The other two previously-raised optional suggestions (the causal-reasoning nuance in bullet 1, and the generic #rem-name placeholder) remain unaddressed, which is fine since both were explicitly non-blocking stylistic suggestions.

No new issues in prose, Markdown syntax, cross-references, or SERG lab-manual conventions (no raw HTML, markdown-only, US spelling) — this range touches only the one class token.

Verdict

Ready for merge — no blocking issues. One non-blocking finding carried over and re-verified against quarto-cli source: the reintroduced .remark class is redundant, and the commit's stated justification for keeping it does not match actual rendering behavior.

Structured Review Data (JSON)
{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "fcc47da2dcccf00d338011aad77fae555a5e4e0e",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "chapters/teaching.qmd line 163 re-added the .remark class that a prior round removed; quarto-cli's mainstateinit.lua registers 'rem' as a built-in crossref category so the #rem- prefix alone already triggers is_proof_div, making the class redundant rather than required.",
  "holistic_assessment": "Scope stays limited to one class token with no regression risk to rendering, but reverts a previously validated consistency fix against using-theorem-environments.qmd without new evidence, so leaving it as-is is a documentation-consistency nit rather than a blocking integration concern."
}

Reviewed commit: ab64dbb

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

💰 Cost: $0.8710 (review) — run

@d-morrison
d-morrison merged commit 358f54c into main Oct 8, 2026
15 checks passed
@d-morrison
d-morrison deleted the claude/project-thread-hzfopq branch October 8, 2026 05:03
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-08 05:06 UTC

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