Skip to content

Box ch. 6 content into numbered divs and callouts - #119

Open
d-morrison wants to merge 3 commits into
mainfrom
claude/ch06-divs
Open

d-morrison wants to merge 3 commits into
mainfrom
claude/ch06-divs

Conversation

@d-morrison

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

Copy link
Copy Markdown
Collaborator

Requested by Ezra · project thread

Part of #77: ch. 6 (graphical representation of causal effects).

Before: ch. 6 had only a few divs (causal Markov, path, collider, systematic bias, and five examples). Most terms were bold words in prose or sat inside Technical Point and Fine Point callouts. Several were used before they were defined. For example, "parents" and "non-descendants" appeared in the causal Markov definition but were defined only later, in Technical Point 6.1.

After: 57 numbered boxes, each defined before first use. Each new definition is followed by a concrete example.

  • 18 definitions: DAG; parents, descendants and ancestors; causal DAG; Markov property; NPSEM; NPSEM-IE; FFRCISTG; blocked and open paths; d-separation; faithfulness; causal discovery; unconditional and conditional bias; bias under the null; causal and surrogate effect modifiers.
  • 3 propositions and 1 theorem:
    • the Markov factorization, with a proof in both directions;
    • every NPSEM-IE is an FFRCISTG but not conversely, with a proof and a counterexample I wrote (an XOR of two fair coins);
    • FFRCISTGs imply the causal Markov assumption (cited to Robins 1986);
    • Pearl's d-separation theorem, with its conditions stated.
  • 27 examples. New ones include unconditional bias without conditional bias in Table 3.1, worked from the table's counts.
  • 7 remarks and 1 table.
  • Warning, tip and note callouts.

No theorem div sits inside a callout or .notes. The Technical Point and Fine Point callouts now hold short summaries and point to the boxes that follow them, as ch. 12 does.

How: I wrapped the existing text, following the merged ch. 8 (#109) and ch. 10 (#110) passes and the review findings they received.

  • Claims about collider stratification are now generic ("for all but special parameter values"). They promise association only "in at least one level". A warning says the diagram does not fix the sign of that association.
  • Identification claims now list consistency and positivity.
  • Passages the adversarial review found too close to the book were reworded: the matching example, the mineral-water note and the dose-finding notes.
  • I kept the heading anchors that ch. 8 and ch. 19 link to (#bias-under-the-null, #from-d-separation-to-independence).
  • All new ids are unique across the book.

Checks:

  • quarto render chapters/06-graphical-representation.qmd --to html succeeds with no warnings, and the page has no unresolved ?@ references.
  • A script confirmed that every crossref resolves, that no reference points forward, that no id is duplicated across chapters, and that the divs balance.
  • The gha semantic-line-break check passes on the added lines.
  • The spell check finds no new words, so inst/WORDLIST is unchanged.
  • An OpenCode adversarial review (gpt-5.6-luna) ran on 7cc9476 and reported 8 findings. All 8 are fixed in 2a8417a; the PR comment lists them. A second round could not run because the OpenCode Go quota is exhausted, and the Claude review in CI hit a 429. This PR has no clean external verdict yet.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Gh7L9vJhwrpvpBhzYMdjPK

claude added 2 commits October 9, 2026 08:58
Put chapter 6's definitions, results, examples, remarks, cautions and
tips into crossref divs and callouts. Definitions now precede first use
(DAG, parents and descendants, causal DAG, Markov property, NPSEM,
NPSEM-IE, FFRCISTG, blocked paths, d-separation, faithfulness, causal
discovery, the bias types, surrogate effect modifiers), each followed by
a concrete example. New propositions state the Markov factorization
(with proof), that every NPSEM-IE is an FFRCISTG but not conversely
(with proof and a counterexample), and that both models imply the
causal Markov assumption; Pearl's d-separation theorem is boxed with
its conditions. Collider-conditioning claims are hedged as generic,
with a warning that the diagram does not fix the sign.

Part of #77.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gh7L9vJhwrpvpBhzYMdjPK
…ations, reword close paraphrases

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gh7L9vJhwrpvpBhzYMdjPK
Copilot AI balanced review requested due to automatic review settings October 9, 2026 09:21

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@d-morrison d-morrison self-assigned this Oct 9, 2026
@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Collaborator Author

Adversarial review (OpenCode, gpt-5.6-luna), round 1 on 7cc9476: 8 findings, not clean. Each one is addressed in 2a8417a:

  1. Identification claims lacked consistency. Fixed: exm-discovery and Fine Point 6.4 now require consistency and positivity, as well as a well-defined $Y^a$.
  2. "Condition 3 makes causal Markov credible" overclaimed, and the summary called the common-cause requirement "the causal Markov assumption". Fixed: both now say the requirement is needed for the causal Markov assumption.
  3. Collider associations were not hedged enough. Fixed: the conditioning table and the diuretic example now say the association holds generically (for all but special parameter values) in at least one level of the conditioning variable.
  4. def-npsem wrote $V_m = f_m(\epsilon_m)$ while naming $V_1$. Fixed: the sentence now names $V_m$ before $V_1$ is given as the example.
  5. A .notes block sat inside the Fine Point 6.4 callout. This was already on main, but it is fixed anyway: the notes now follow the callout. The chapter now has no nested divs.
    6-8. The matching example, the mineral-water note and the Fine Point 6.4 dose-finding notes were too close to the book. All three are rewritten in our own words and order.

Round 2 could not run. Every OpenCode Go model (gpt-5.6-luna, kimi-k3, deepseek-v4-pro) now fails with Go usage limit exceeded, and the Claude review in CI hit a 429 (resets 12:20 UTC). This PR therefore has no clean external verdict yet. The Claude review needs re-triggering after the reset.

Local checks on 2a8417a:

  • The HTML render succeeds with 0 warnings and no unresolved ?@ references.
  • Crossrefs resolve with no forward references, and no id is duplicated across chapters.
  • The divs balance.
  • The semantic-line-break check is clean.
  • The spell check finds no new words.

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

/review


Generated by Claude Code

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Collaborator Author

Rebutting the one finding in the latest Claude review (FFRCISTGs / NPSEM-IEs may fail the spell check): the Spellcheck job already ran and passed on this exact head (2a8417a), so those plural forms are accepted by the project's spellcheck and inst/WORDLIST needs no change.
No code change; re-dispatching the review so its verdict reflects that.

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

/review

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gh7L9vJhwrpvpBhzYMdjPK
Copilot AI balanced review requested due to automatic review settings October 9, 2026 10:11

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Claude finished review — View run


Review: PR #119 — Box ch. 6 content into numbered divs and callouts

This round reviews the range computed by the workflow, 2a8417ad..56114369, which consists of exactly one commit (403bc16) touching one file with a single-line change in chapters/06-graphical-representation.qmd.

The fix for the sole outstanding finding has been applied and verified correct.

The prior two review rounds (round 1, round 2) flagged chapters/06-graphical-representation.qmd:444 (then-current line): "The converse fails: some FFRCISTGs are not NPSEM-IEs." — introducing plural acronyms (FFRCISTGs, NPSEM-IEs) absent from inst/WORDLIST, likely failing the check-spelling CI job.

This commit rewords the sentence to:

"The converse fails: some models that satisfy the FFRCISTG condition are not NPSEM-IE models." (chapters/06-graphical-representation.qmd:431)

I checked this directly:

  • FFRCISTG (singular) — present in inst/WORDLIST:75.
  • NPSEM-IE (singular, hyphenated) — splits at the hyphen into NPSEM (inst/WORDLIST:187) and IE (inst/WORDLIST:102), both already listed. This matches the same hyphen-splitting logic the prior rounds used to explain why NPSEM-IEs failed (it split into the unlisted IEs), confirming the singular form here passes.
  • No new plural forms are introduced; inst/WORDLIST is correctly left unchanged, since no new words were added.
  • The sentence still carries the same mathematical content (the converse of @prp-npsem-ie-ffrcistg fails) with no loss of meaning — "models that satisfy the FFRCISTG condition... are not NPSEM-IE models" is an accurate paraphrase of "some FFRCISTGs are not NPSEM-IEs."

No other content changed in this range, and the rest of the diff (crossrefs, proofs, citations, div structure, macro usage) was already verified clean across the two prior rounds and remains unchanged here.

No d-morrison/macros opportunities apply to this one-line prose edit; no new/changed math was introduced.

No new issues found.

Verdict

Ready for merge — the single outstanding issue from prior rounds (likely CI spellcheck failure from unlisted plurals) has been resolved by rewording to avoid the plural acronyms, verified against inst/WORDLIST; nothing else in this range raises a concern.

Structured Review Data (JSON)
{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "56114369d2cb10db26f3f6ddb8c476345a761a51",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "chapters/06-graphical-representation.qmd:431 now reads 'some models that satisfy the FFRCISTG condition are not NPSEM-IE models', avoiding the unlisted plural acronyms FFRCISTGs and NPSEM-IEs that would have failed check-spelling CI in prior rounds.",
  "holistic_assessment": "Validation scope is limited to this single-line wording fix; wordlist entries for FFRCISTG, NPSEM, and IE confirm the singular forms used now resolve cleanly with no regression to crossref or proof content reviewed earlier."
}

Reviewed commit: 403bc16

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

💰 Cost: $0.1946 (review) — run

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://Morrison-Lab.github.io/win/pr-preview/pr-119/

Built to branch gh-pages at 2026-10-09 10:27 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

This branch has not been deployed

No deployments
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.

3 participants