Skip to content

[WIP] Add section to logistic regression on computing marginal risk differences#706

Open
Claude wants to merge 6 commits into
mainfrom
claude/add-logistic-regression-marginal-risk
Open

[WIP] Add section to logistic regression on computing marginal risk differences#706
Claude wants to merge 6 commits into
mainfrom
claude/add-logistic-regression-marginal-risk

Conversation

@Claude

@Claude Claude AI commented May 12, 2026

Copy link
Copy Markdown
Contributor

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.


This section details on the original issue you should resolve

<issue_title>add a section to logistic regression explaining how to compute marginal risk differences from logistic models</issue_title>
<issue_description>include how to get standard errors and confidence intervals using the bootstrap

include a numerical example</issue_description>

Comments on the Issue (you are @claude[agent] in this section)

@Claude Claude AI requested a review from d-morrison May 12, 2026 22:20
@d-morrison d-morrison marked this pull request as ready for review May 13, 2026 03:18
Copilot AI review requested due to automatic review settings May 13, 2026 03:18
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd

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

This PR (WIP) expands the logistic regression materials by adding an explicit section on computing marginal risk differences (and risk ratios) from logistic models via predictive margins / g-computation, including bootstrap-based uncertainty estimation and a worked WCGS example.

Changes:

  • Adds conceptual explanation of conditional vs marginal RD/RR from a logistic model, with notation and formulas.
  • Adds a worked WCGS example that computes a marginal RD, a bootstrap SE/CI, and visualizes the bootstrap distribution.
  • Adds alternative implementations using boot::boot() and extends the approach to marginal risk ratios.

Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
@d-morrison

Copy link
Copy Markdown
Owner

@copilot apply changes based on the comments in this thread

Copilot AI commented May 13, 2026

Copy link
Copy Markdown
Contributor

@copilot apply changes based on the comments in this thread

Implemented in 2cfbf9a. I applied the thread feedback: adjusted heading depth for subfile hierarchy, separated marginal estimand vs estimator notation, added the requested numerical point-estimate example, made exposure-level handling explicit/validated in helper code, and reduced rendered bootstrap iterations with guidance for higher counts in final analyses. Screenshot: /tmp/logistic-regression-updates.png.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • accounts.google.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=12982 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.92q5O1 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,16865911708213916311,9884497677581521791,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,1197906380075195154,7778487634538802180,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=38281 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpjxEVcY/chrome-32817b278846-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio (dns block)
  • android.clients.google.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=12982 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.92q5O1 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,16865911708213916311,9884497677581521791,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,1197906380075195154,7778487634538802180,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=38281 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpjxEVcY/chrome-32817b278846-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio (dns block)
  • clients2.google.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=12982 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.92q5O1 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,16865911708213916311,9884497677581521791,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,1197906380075195154,7778487634538802180,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=38281 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpjxEVcY/chrome-32817b278846-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio (dns block)
  • redirector.gvt1.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=12982 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.92q5O1 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,16865911708213916311,9884497677581521791,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,1197906380075195154,7778487634538802180,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=38281 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpjxEVcY/chrome-32817b278846-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio (dns block)
  • safebrowsingohttpgateway.googleapis.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=12982 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.92q5O1 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,16865911708213916311,9884497677581521791,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,1197906380075195154,7778487634538802180,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=38281 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpjxEVcY/chrome-32817b278846-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.

Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
@d-morrison

Copy link
Copy Markdown
Owner

@copilot apply changes based on the comments in this thread and #706 (review)

check the section heading levels to make sure there are no jumps (ignore div title headings)

Copilot AI commented May 13, 2026

Copy link
Copy Markdown
Contributor

@copilot apply changes based on the comments in this thread and #706 (review)...

Implemented in 1ec2533. I applied both linked thread requests and rechecked regular section heading levels for no jumps (ignoring div-title headings). Screenshot: /tmp/logistic-regression-latest.png.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • accounts.google.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=16078 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.WmxAjs --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,2727516718192104785,1615619502188767473,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,12364519729979651043,12519947518436324700,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=44483 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpCxSj7V/chrome-3e996275c1ea-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio /usr/local/.ghcup/bin/sh bash 51a7�� --noprofile sh ndor/bin/sh (dns block)
  • android.clients.google.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=16078 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.WmxAjs --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,2727516718192104785,1615619502188767473,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,12364519729979651043,12519947518436324700,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=44483 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpCxSj7V/chrome-3e996275c1ea-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio /usr/local/.ghcup/bin/sh bash 51a7�� --noprofile sh ndor/bin/sh (dns block)
  • clients2.google.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=16078 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.WmxAjs --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,2727516718192104785,1615619502188767473,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,12364519729979651043,12519947518436324700,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=44483 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpCxSj7V/chrome-3e996275c1ea-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio /usr/local/.ghcup/bin/sh bash 51a7�� --noprofile sh ndor/bin/sh (dns block)
  • redirector.gvt1.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=16078 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.WmxAjs --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,2727516718192104785,1615619502188767473,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,12364519729979651043,12519947518436324700,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=44483 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpCxSj7V/chrome-3e996275c1ea-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio /usr/local/.ghcup/bin/sh bash 51a7�� --noprofile sh ndor/bin/sh (dns block)
  • safebrowsingohttpgateway.googleapis.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --mute-audio --crashpad-handler-pid=16078 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/com.google.Chrome.scoped_dir.WmxAjs --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,2727516718192104785,1615619502188767473,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,12364519729979651043,12519947518436324700,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --headless --remote-debugging-port=44483 --remote-allow-origins=REDACTED --crash-dumps-dir=/tmp/RtmpCxSj7V/chrome-3e996275c1ea-crashpad --no-sandbox --disable-dev-shm-usage --force-color-profile=srgb --disable-extensions --mute-audio /usr/local/.ghcup/bin/sh bash 51a7�� --noprofile sh ndor/bin/sh (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated

@d-morrison d-morrison left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clearly distinguish between

Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
Comment thread _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd Outdated
@github-actions github-actions Bot deleted a comment from claude Bot May 25, 2026
@github-actions github-actions Bot deleted a comment from claude Bot May 25, 2026
@claude

claude Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 5m 3s —— View job


Code Review

  • Gather context: read changed files
  • Review content and structure
  • Check math notation and correctness
  • Check heading levels
  • Check code quality
  • Post review findings

Summary

The PR is in good shape overall — the math, notation, and code structure are solid. Three issues need attention before this comes out of WIP.


Bug: entire "Notes and caveats" content is hidden in HTML

File: _subfiles/logistic-regression/_sec-logistic-marginal-considerations.qmd

The entire file is wrapped in ::: notes (RevealJS presenter-notes syntax, L1 / L61). That content is invisible in HTML output — it only appears during slide presentations. This includes:

  • Target population caveats
  • Collapsibility explanation with formulas
  • Bootstrap sample-size guidance (≥1000 for SEs, ≥2000 for CIs)
  • margins package reference
  • Case-control study warning

These are all pedagogically important for students reading the textbook. The ::: notes wrapper should be changed to a {.callout-note} block or regular paragraphs. Fix this →

The same issue applies to the citation block at the top of _sec-bootstrap-inference.qmd (L1–7), which is also in ::: notes. The citations for Vittinghoff, Hastie, and James won't appear in HTML. Those should probably be inline citations in regular prose or a footnote.


Bug: duplicate chunk + div labels (previously flagged, not yet fixed)

File: _subfiles/logistic-regression/_sec-wcgs-rd-bootstrap.qmd, L59–62 and _sec-wcgs-rd-viz.qmd L4–7

Both files have matching {#tbl-...} / {#fig-...} div labels and #| label: tbl-... / #| label: fig-... chunk labels with the same identifier. From the project conventions, the chunk label should be removed (or use a non-tbl-/fig--prefixed label) and rely solely on the div id for cross-references. Fix this →


Structural question: heading isolated between two slidebreaks

File: _subfiles/logistic-regression/_sec-logistic-RR-RD.qmd, L80–86

{{< slidebreak >}}     ← L80

### Notes and caveats  ← L82

{{< slidebreak >}}     ← L84

{{< include ..._sec-marginal-notes.qmd >}}

The heading appears alone on one slide; the actual content is pushed to the next slide. If this is intentional (section-title slide style), it's fine — just confirming it's deliberate and not a leftover from the earlier slidebreak-ordering fixes.


Positive: math is correct

  • #thm-observed-marginal-risk: correctly derives $\pi(a) = \E{\pi(a,Z) \mid A=a}$ using $f_{Z\mid A}$ as weights. Color-coding with \blue/\red is clear.
  • #thm-causal-marginal-risk: correctly derives $\pi_a = \E{\pi(a,Z)}$ using the marginal $f_Z$ under consistency + conditional exchangeability + positivity. The two theorems cleanly illustrate why the observed and causal estimands differ.
  • #exm-collapsibility: numerical demonstration that RD is collapsible but RR and OR are not is correct and well-structured.
  • #def-g-computation and #def-predictive-margins are clearly distinguished (full-sample vs. subgroup averaging).

Positive: cross-chapter references verified

All div IDs referenced across chapters exist in their targets:

  • def-potential-outcomes, def-consistency, def-exchangeability, def-positivity — confirmed present in _subfiles/causal-inference/

Positive: code quality

  • compute_marginal_rd validates factor levels with an informative error message — no longer fragile positional indexing.
  • invisible(load(...)) suppresses spurious console output.
  • Bootstrap is clearly labeled as illustrative (B=300) with explicit guidance to use ≥2000 for final analyses.
  • All numerical results use inline R — no hard-coded numbers.
  • _sec-marginal-point-estimate.qmd computes both observed and g-computation estimates in the same chunk and notes the divergence, directly showing students why standardization matters.

Blocking before WIP removal: items 1 (::: notes visibility) and 2 (duplicate labels). Item 3 (isolated heading slide) needs author confirmation.

@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Code Review — PR #706

  • Gather diff and context
  • Run three-angle analysis (line-by-line, removed behavior, cross-file)
  • Verify candidates
  • Post findings

View job run

@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 5m 15s —— View job


Code Review — PR #706

  • Read changed files
  • Review math/notation correctness
  • Check heading levels and structure
  • Check Quarto conventions (divs, slidebreaks, etc.)
  • Check R code quality
  • Check cross-references

Bugs (blocking)

1. _sec-logistic-marginal-considerations.qmd L1–61: entire content is hidden in HTML

The file is wrapped in ::: notes (RevealJS presenter-notes syntax). Everything inside is invisible in HTML output — only visible during slide presentations. This includes:

  • Target population caveat
  • Collapsibility discussion with formulas for RD/RR/OR
  • Bootstrap sample-size guidance (≥1000 for SEs, ≥2000 for CIs)
  • margins package link
  • Case-control warning

This file is included by _sec-marginal-notes.qmd at line 19. The ::: notes wrapper must be replaced with regular prose (or a {.callout-note} block if you want it visually distinguished). Fix this →

2. _sec-bootstrap-inference.qmd L1–7: citations hidden in HTML

The references to Vittinghoff, Hastie, and James (lines 1–7) are wrapped in ::: notes. They will not appear in HTML output. Convert to inline citations in regular prose, e.g.:

Adapted from @vittinghoff2e [Section 3.6, p. 62],
@hastie2009esl2e [Section 7.11, p. 249],
and @james2021islr2e [Chapter 5, Section 5.2, p. 209].

Fix this →

3. Duplicate chunk + div labels (two files)

  • _sec-wcgs-rd-bootstrap.qmd L59 and L62: {#tbl-marginal-rd-results} div label and #| label: tbl-marginal-rd-results chunk label are the same. Per project convention, remove the chunk-level label: when a div id is used. Fix →
  • _sec-wcgs-rd-viz.qmd L4 and L7: Same issue with {#fig-bootstrap-dist} and #| label: fig-bootstrap-dist. Fix →

Structural issues (author confirmation requested)

4. _sec-logistic-RR-RD.qmd L80–86: heading isolated between slidebreaks

{{< slidebreak >}}      ← L80
### Notes and caveats   ← L82
{{< slidebreak >}}      ← L84
{{< include _sec-marginal-notes.qmd >}}

The heading appears alone on one slide; the actual content starts on the next. If intentional (section-title slide), it's fine — but if the goal is heading + content on the same slide, move the L80 {{< slidebreak >}} to after the include on L86 and remove L84.


Positive findings

Math — all correct:

  • #thm-observed-marginal-risk: correctly derives $\pi(a) = \E{\pi(a,Z) \mid A=a}$ using $f_{Z \mid A}$ as weights (law of total probability). Color-coding \blue/\red cleanly distinguishes shared vs. focal terms.
  • #thm-causal-marginal-risk: correctly derives $\pi_a = \E{\pi(a,Z)}$ using marginal $f_Z$ under consistency + conditional exchangeability + positivity. The two theorems together illustrate precisely why observed and causal estimands differ.
  • #def-collapsibility: rigorous definition. Correctly states that collapsibility bias = $\theta - \E{\theta(Z)}$.
  • #thm-collapsibility: correct — additive measures collapsible by linearity of expectation; ratio and OR measures not.
  • #exm-collapsibility L49–61: correctly notes non-collapsibility ≠ effect modification, and confirms that a constant conditional OR would still produce a marginal OR differing from it.

Notation discipline: Observed vs. causal estimands are clearly distinguished throughout. \pi(a) uses $f_{Z \mid A}$ weights; `\pi_a$ uses marginal $f_Z$. The two proofs visually contrast via color-coding.

#def-g-computation vs #def-predictive-margins: Properly distinguished — g-computation averages over the full sample; predictive margins averaged over ${A_i = a}$ estimates the observed marginal risk. This is pedagogically clear.

R code quality:

  • compute_marginal_rd validates exposure factor levels with an informative error message — no fragile positional indexing.
  • invisible(load(...)) suppresses spurious print output.
  • B <- 300 is clearly labeled "illustrative only; use ≥ 2000 for final CIs" at multiple points.
  • All numerical results in prose use inline R — no hardcoded values.

Cross-chapter references verified: def-consistency, def-exchangeability, and def-positivity all exist in _subfiles/causal-inference/.

_sec-marginal-point-estimate.qmd: Computes both observed ($\hat\pi(a)$) and g-computation ($\hat\pi_a$) estimates in the same chunk and explicitly notes their divergence — a concrete illustration of why standardization matters.


Blocking before WIP removal: issues 1 (::: notes content hidden), 2 (citations hidden), and 3 (duplicate labels). Issue 4 needs author confirmation.

@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

Initial WIP draft of a section on computing marginal risk
differences from logistic regression models, with bootstrap
standard errors and a numerical example.

Squashed from the original 87-commit history because the action's
recursive submodule fetch was failing on commits that referenced
a now-deleted latex-macros SHA. See the PR description for context.
@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 3m 36s —— View job


Code Review — PR #706

  • Read changed files and gather context
  • Review math/notation correctness
  • Check heading levels and structure
  • Check Quarto conventions (divs, slidebreaks, ::: notes usage)
  • Check R code quality
  • Check cross-references
  • Post inline review comments and summary

Three bugs flagged in earlier reviews remain unaddressed. Everything else looks solid.


Bugs (blocking — same issues as the 2026-05-25 and 2026-05-26 reviews; still unresolved)

1. _sec-logistic-marginal-considerations.qmd L1 & L61 — entire content hidden in HTML

The file is wrapped in ::: notes (RevealJS presenter-notes syntax). Nothing inside is visible in HTML output. This hides:

  • The target-population caveat (item 1)
  • The collapsibility discussion with RD/RR/OR formulas (item 2)
  • Bootstrap sample-size guidance (≥ 1000 for SEs, ≥ 2000 for CIs) (item 3)
  • The margins package CRAN link (item 4)
  • The case-control studies warning (item 5)

Remove the ::: notes wrapper and use regular prose or a {.callout-note} block. If there is also no {{< slidebreak >}} between _thm-collapsibility.qmd and this include (there isn't currently), add one at the same time. Fix this →

2. _sec-bootstrap-inference.qmd L1–7 — citations hidden in HTML

The three references (Vittinghoff, Hastie, James) are wrapped in ::: notes. They will not appear in HTML. Replace with inline prose:

Adapted from @vittinghoff2e [Section 3.6, p. 62],
@hastie2009esl2e [Section 7.11, p. 249],
and @james2021islr2e [Chapter 5, Section 5.2, p. 209].

Fix this →

3. Duplicate chunk + div labels (two files)

  • _sec-wcgs-rd-bootstrap.qmd L59 and L62: {#tbl-marginal-rd-results} is the div label (L59) and #| label: tbl-marginal-rd-results is an identical chunk label (L62). Per project convention (div id is authoritative; chunk label should not duplicate it with a tbl- prefix). Remove the #| label: tbl-marginal-rd-results chunk option. Fix →

  • _sec-wcgs-rd-viz.qmd L4 and L7: same issue with {#fig-bootstrap-dist} and #| label: fig-bootstrap-dist. Remove the chunk label. Fix →


Structural question (author confirmation)

4. _sec-logistic-RR-RD.qmd L80–86 — heading isolated between two slidebreaks

{{< slidebreak >}}       ← L80
### Notes and caveats    ← L82
{{< slidebreak >}}       ← L84
{{< include _sec-marginal-notes.qmd >}}

The heading appears alone on one slide; all content starts on the next. This was flagged in the prior review. If it is intentional (section-title slide), no change needed — please confirm so the thread can be resolved.


Positive findings

Math — all correct:

  • #thm-observed-marginal-risk: correctly derives $\pi(a) = \mathbb{E}[\pi(a,Z) \mid A=a]$ using $f_{Z\mid A}$ as weights; color-coding \blue/\red cleanly marks shared vs. focal terms.
  • #thm-causal-marginal-risk: correctly derives $\pi_a = \mathbb{E}[\pi(a,Z)]$ under consistency + conditional exchangeability + positivity using marginal $f_Z$. The side-by-side structure of the two proofs makes the distinction between observed and causal estimands immediately visible.
  • #def-collapsibility: well-formulated; not restricted to "measures of association" — applies to any estimand $\theta$. #thm-collapsibility correctly extends this to both causal (marginal $f_Z$) and observational ($f_{Z\mid A}$) contexts.
  • #exm-collapsibility: the two-strata numerical illustration is correct; the final paragraph correctly notes that non-collapsibility is distinct from effect-measure modification.

Notation discipline: Observed ($\hat\pi(a)$, weighted by $f_{Z\mid A}$) vs. causal ($\hat\pi_a$, weighted by $f_Z$) estimands are cleanly distinguished throughout, including in the point-estimate example which explicitly displays both and notes they differ.

#def-g-computation vs. #def-predictive-margins: well distinguished — g-computation averages over the full sample; predictive margins (subgroup average) estimates the observed marginal risk. This is clear and pedagogically correct.

R code quality:

  • compute_marginal_rd / make_counterfactual_datasets validate factor levels with an informative error message — no fragile positional indexing.
  • invisible(load(...)) suppresses spurious print output in the setup chunk.
  • B <- 300 is clearly labeled "illustrative only; use ≥ 2000 for final CIs" in multiple places, and _sec-wcgs-rd-interpretation.qmd repeats this caveat in the interpretation paragraph.
  • All numerical results in prose use inline R — no hard-coded values.

Cross-chapter references: def-consistency, def-exchangeability, and def-positivity all exist in the causal-inference subfiles — verified.

Heading levels: No jumps. The chain # → ## → ### → #### → ##### is respected across the include hierarchy (ignoring div-title headings as instructed).


Must fix before merging: issues 1, 2, 3. Issue 4 needs a confirming comment from the author.

claude added 2 commits June 9, 2026 07:59
- _sec-logistic-marginal-considerations.qmd:1,61: remove ::: notes
  wrapper around the entire considerations list. This content is
  primary teaching material (target population, collapsibility,
  bootstrap sample size, alternative methods, case-control caveat),
  not asides — hiding it inside ::: notes excludes it from the
  rendered HTML body.
- _sec-bootstrap-inference.qmd:1-7: same — the citation block is a
  source attribution, not a margin note.
- _sec-wcgs-rd-bootstrap.qmd:62, _sec-wcgs-rd-viz.qmd:7: remove the
  duplicate `#| label:` line from each chunk. The label is already
  supplied by the enclosing `:::{#tbl-...}` / `:::{#fig-...}` div;
  the duplicate triggers a Quarto warning ("label clashes with div").
@claude

claude Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 6m 19s —— View job


Code Review — PR #706

  • Read changed files and gather context
  • Review math/notation correctness
  • Check heading levels and structure
  • Check Quarto conventions (divs, slidebreaks, ::: notes usage)
  • Check R code quality
  • Check cross-references
  • Post review findings

All Three Previously-Blocking Issues Are Fixed

  1. _sec-logistic-marginal-considerations.qmd::: notes wrapper removed; all five considerations (target population, collapsibility, bootstrap sample size, margins package, case-control warning) are now visible in HTML output. ✅
  2. _sec-bootstrap-inference.qmd L1–5 — Vittinghoff/Hastie/James citations are now inline prose, not inside ::: notes. ✅
  3. Duplicate chunk + div labels#| label: tbl-marginal-rd-results removed from _sec-wcgs-rd-bootstrap.qmd; #| label: fig-bootstrap-dist removed from _sec-wcgs-rd-viz.qmd. Both files now rely solely on the div id. ✅

One Remaining Structural Question

_sec-logistic-RR-RD.qmd L80–86### Notes and caveats is still isolated between two slidebreaks:

{{< slidebreak >}}       ← L80
### Notes and caveats    ← L82  (its own slide, no content)
{{< slidebreak >}}       ← L84  (required before #def-collapsibility per new policy)
{{< include _sec-marginal-notes.qmd >}}

This has been flagged in every review since 2026-05-19 without author resolution. The current structure is actually consistent with the new CLAUDE.md policy (slidebreak before every thm-type div — here the #def-collapsibility div that opens _sec-marginal-notes.qmd). The heading becomes a deliberate section-title slide.

Author action needed: Please confirm this is intentional, or remove either the L80 or L84 slidebreak. Once confirmed, this thread can be resolved.


New Finding: Ordering Dependency Is Implicit

_sec-marginal-point-estimate.qmd L53–68 and _sec-wcgs-rd-compute.qmd: The point-estimate file creates counterfactual datasets inline (wcgs_counterfactual_a, wcgs_counterfactual_b) without the make_counterfactual_datasets() helper — because that helper is defined later in _sec-wcgs-rd-compute.qmd. This is not a bug (chunks execute in document order), but a reader encountering the two files out of context would see the same operation expressed differently. Not blocking, but something to be aware of.


Positive Findings

Math — all correct and clean:

  • #thm-observed-marginal-risk: derives $\pi(a) = \mathbb{E}[\pi(a,Z) \mid A=a]$ via the law of total probability. Color-coding \blue/\red marks shared vs. focal terms clearly.
  • #thm-causal-marginal-risk: derives $\pi_a = \mathbb{E}[\pi(a,Z)]$ under consistency + conditional exchangeability + positivity, correctly using marginal $f_Z$ (not $f_{Z \mid A}$).
  • #def-collapsibility + #thm-collapsibility: rigorous and correct. Correctly treats both causal ($f_Z$ weights) and observational ($f_{Z\mid A}$ weights) cases. Collapsibility bias term defined on _sec-marginal-notes.qmd L11.
  • #exm-collapsibility: numerical table shows RD collapses, RR/OR do not. Last paragraph correctly distinguishes non-collapsibility from effect-measure modification. The "two assumptions together imply..." framing is correct (neither assumption alone is sufficient).

Notation discipline — observed vs. causal kept separate throughout:

  • $\pi(a)$ uses $f_{Z\mid A}$ weights; $\pi_a$ uses marginal $f_Z$. The two theorems make this contrast visible.
  • #def-causal-marginal-risk (L77) includes "potential marginal risk" as a synonym. ✓
  • #exm-marginal-point-estimate computes both $\hat\pi(a)$ (raw) and $\hat\pi_a$ (g-computation) and explicitly notes they differ. ✓

Definitions/theorems cleanly separated:

  • #def-observed-marginal-risk: definition only (probability), not the identification formula.
  • #thm-observed-marginal-risk: identification formula as a theorem.
  • Same pattern for #def-causal-marginal-risk / #thm-causal-marginal-risk. ✓

Structural:

  • No heading level jumps. Chain ## → ### → #### → ##### is respected through the include hierarchy (ignoring div-title headings per CLAUDE.md).
  • All theorem-type divs have {{< slidebreak >}} in the including file, not inside the subfile. ✓
  • All subfiles are under 150 lines. ✓
  • "Optional for 2026" callout note present at _sec-logistic-RR-RD.qmd L9–11. ✓

Cross-references verified:

  • def-potential-outcomes ✓ — _subfiles/causal-inference/_sec_potential_outcomes.qmd
  • def-consistency, def-exchangeability, def-positivity ✓ — _subfiles/causal-inference/_sec_assumptions.qmd
  • sec-bootstrap-ci ✓ — chapters/basic-statistical-methods.qmd line 698

R code quality:

  • make_counterfactual_datasets() validates factor levels with an informative error message.
  • invisible(load(...)) suppresses print output from load().
  • B <- 300 clearly labeled "illustrative only; use ≥ 2000 for final CIs" in multiple places.
  • Bootstrap interpretation (_sec-wcgs-rd-interpretation.qmd) avoids hardcoded conclusion; correctly uses inline R and notes the CI is illustrative. ✓
  • eval: false on _sec-bootstrap-boot-package.qmd and _sec-marginal-rr.qmd keeps render time manageable. ✓

CLAUDE.md + WORDLIST updated correctly:

  • New slidebreak-before-thm-type-div policies documented. ✓
  • collapsibility / Collapsibility added to WORDLIST. ✓

Status: All three previously-blocking issues are resolved. The only item needed before WIP removal is author confirmation (or fix) on the isolated ### Notes and caveats heading between two slidebreaks at _sec-logistic-RR-RD.qmd L80–86.

@claude

claude Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 7m 24s —— View job


Code Review — PR #706

  • Gather context: read CLAUDE.md and diff
  • Read all changed files
  • Review math/notation correctness
  • Check heading levels and structure
  • Check Quarto conventions (divs, slidebreaks, ::: notes usage)
  • Check R code quality
  • Check cross-references
  • Post inline review comments and summary

Confirmed resolved (previously blocking)

All three issues flagged in the 2026-05-25 / 2026-05-26 / 2026-06-09 reviews are fixed in commit 0c6ae9a:

  1. ::: notes wrapper removed from _sec-logistic-marginal-considerations.qmd — considerations are visible in HTML. ✅
  2. Citations in _sec-bootstrap-inference.qmd are now inline prose. ✅
  3. Duplicate #| label: chunk options removed from _sec-wcgs-rd-bootstrap.qmd and _sec-wcgs-rd-viz.qmd. ✅

Issues still needing attention

1. \blue color: dark-mode visibility not fixed

Files: _thm-observed-marginal-risk.qmd L7, 9, 22, 24 · _thm-causal-marginal-risk.qmd L9, 31, 33

D-morrison explicitly requested a color change on 2026-05-20 ("blue color doesn't show up well in dark mode; use a different color?"). No commit since then has touched these files. Both theorems still use \blue{...} (resolves to {\color{blue}{...}}). The macro library has \teal, \cyan, \orange — all of which render better against dark backgrounds.

Fix: replace \blue with \teal (or another dark-mode-friendly color) in both theorem files. Fix this →

2. ### Notes and caveats isolated between two slidebreaks (author confirmation needed)

File: _sec-logistic-RR-RD.qmd L80–86

{{< slidebreak >}}       ← L80
### Notes and caveats    ← L82  (sole content on this slide)
{{< slidebreak >}}       ← L84  (required: _sec-marginal-notes.qmd starts with #def-collapsibility)
{{< include _sec-marginal-notes.qmd >}}

The L84 slidebreak is now required by CLAUDE.md policy (slidebreak before every theorem-type div, applied at the including-file level). The L80 slidebreak results in a section-title slide that shows only the heading. Flagged in every review since 2026-05-19 without author confirmation.

If this is not intentional, remove L80's {{< slidebreak >}}. The heading and the L84 slidebreak will still produce a clean section break between "Computing marginal risk ratios" and the collapsibility definitions. Fix this →

3. Informal "collapsible" usage for RR contradicts the formal definition

File: _sec-logistic-marginal-considerations.qmd L21–23

The text says:

Risk ratios are also called collapsible in the sense that they do not exhibit the strong collapsibility bias that odds ratios do

But #thm-collapsibility (just above) formally states that ratio and log-odds measures are not collapsible in general. Students who just read the theorem will find this phrase contradictory. The sentence correctly adds "However, the marginal RR is not in general a weighted average of conditional RRs", but the opening "also called collapsible" still plants confusion.

Suggest either deleting the informal-usage sentence entirely (the math that follows makes the point), or adding an explicit qualifier: "sometimes loosely called collapsible (in an informal sense) in contrast to odds ratios…". Fix this →

4. _sec-marginal-vs-conditional.qmd is 150 lines (minor)

CLAUDE.md: "Aim to keep .qmd source files under ~100 lines; split longer files into named subfiles." This file is 50% over target. Not a hard block, but the causal-estimands definitions (L34–149) could be split into a _sec-causal-marginal-estimands.qmd subfile. Fix this →


All positive findings

Math — all correct:

  • #thm-observed-marginal-risk: correct derivation of $\pi(a) = \E{\pi(a,Z)\mid A=a}$ via law of total probability using $f_{Z\mid A}$ weights. ✅
  • #thm-causal-marginal-risk: correct derivation of $\pi_a = \E{\pi(a,Z)}$ under consistency + conditional exchangeability + positivity, using marginal $f_Z$. ✅
  • #def-collapsibility + #thm-collapsibility: rigorous; correctly treats causal ($f_Z$ weights) and observational ($f_{Z\mid A}$ weights) cases separately. ✅
  • #exm-collapsibility: both assumptions (no confounding AND equal stratum sizes) are stated and correctly applied; the marginal RD = average conditional RD is verified numerically; non-collapsibility ≠ effect modification is correctly noted. ✅

Notation discipline maintained:

  • $\pi(a)$ (observed, $f_{Z\mid A}$ weights) vs. $\pi_a$ (causal, $f_Z$ weights) kept distinct throughout. ✅
  • #def-g-computation vs. #def-predictive-margins: cleanly distinguished (full-sample vs. subgroup average). ✅
  • Observed and causal marginal risk differences (#def-observed-marginal-rd, #def-causal-marginal-rd) separately defined. ✅

Heading levels — no jumps:

  • ############## chain respected through the include hierarchy (div-title headings ignored per CLAUDE.md). ✅

Slidebreaks — all correct (except item 2 above):

  • Slidebreak before every #def-, #thm-, #exm- div, placed in the including file not the subfile. ✅

R code quality:

  • make_counterfactual_datasets() validates factor levels with an informative error message. ✅
  • invisible(load(...)) suppresses stray output. ✅
  • B <- 300 consistently labeled "illustrative only; use ≥ 2000 for final CIs". ✅
  • All numerical results use inline R — no hardcoded values. ✅
  • eval: false on _sec-bootstrap-boot-package.qmd and _sec-marginal-rr.qmd keeps render time manageable. ✅

Cross-references verified:

  • def-potential-outcomes_subfiles/causal-inference/_sec_potential_outcomes.qmd
  • def-consistency, def-exchangeability, def-positivity_subfiles/causal-inference/_sec_assumptions.qmd
  • sec-bootstrap-cichapters/basic-statistical-methods.qmd line 698 ✅

Must fix before merging: items 1 (blue color) and 3 (informal "collapsible" wording). Item 2 needs author confirmation; item 4 is optional cleanup.

claude added 2 commits June 19, 2026 08:09
- replace \blue with \teal in the marginal-risk theorems for dark-mode
  readability (as requested in review)
- fix the collapsibility wording: risk ratios are NOT collapsible in
  general (consistent with thm-collapsibility), with odds ratios
  non-collapsible in an even stronger sense -- removing the incorrect
  'risk ratios are also called collapsible' claim
- drop the slidebreak between the 'Notes and caveats' heading and its
  content so the heading no longer renders as a title-only slide

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011CqGbHzLqKVGnbEpLEdadG

Copy link
Copy Markdown
Owner

Addressed the latest review (commit a477ff8, main merged in):

  • Address (must-fix) — replaced \blue\teal in _thm-observed-marginal-risk.qmd and _thm-causal-marginal-risk.qmd for dark-mode readability (as you'd requested on 2026-05-20).
  • Address (must-fix) — fixed the collapsibility wording in _sec-logistic-marginal-considerations.qmd. It previously claimed "Risk ratios are also called collapsible," which contradicts @thm-collapsibility ("Ratio and log-odds measures are not collapsible in general") and the inequality shown two lines later. Reworded to: risk ratios are not collapsible in general either (marginal RR ≠ weighted average of conditional RRs), with odds ratios non-collapsible in an even stronger sense (marginal OR can differ from every conditional OR even with no confounding).
  • Address (author-decision item) — the ### Notes and caveats heading in _sec-logistic-RR-RD.qmd was sandwiched between two slidebreaks, producing a title-only slide. I removed the slidebreak between the heading and its content (kept the one before it), so the heading now starts a fresh slide together with the notes it introduces. If you'd instead prefer the heading merged onto the preceding "marginal risk ratios" slide, say the word and I'll move the removal.
  • Defer (optional) — _sec-marginal-vs-conditional.qmd is 149 lines (> the ~100 soft target). Since this PR is still [WIP] and that file may change further, I've left the suggested split (factor out the causal-estimands content) for the ongoing WIP pass rather than churn it now. Happy to do it here if you'd prefer.

Checks: lintr::lint() ✓ on all four files; spelling::spell_check_package() ✓; the edited subfiles render cleanly (verified via an isolated include wrapper — all four edits are LaTeX/prose/slide-structure only, no R code touched).

@claude please re-review.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

👀 Picked up by workflow run #27813945839. R/Quarto/renv setup runs first (~3-5 min); Claude itself responds after that.

@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

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


Code Review — PR #706

  • Read changed files and gather context
  • Review the three addressed items from prior review
  • Review math/notation correctness
  • Check heading levels and structure
  • Check Quarto conventions (divs, slidebreaks)
  • Check R code quality
  • Post findings

All three blocking items are confirmed fixed

1. \teal color (dark-mode fix) — ✅

Both theorem files now use \teal throughout:

  • _thm-observed-marginal-risk.qmd: \teal{\pi(a, Z)} for the conditional risk function, \red{A = a} and \red{\p(Z=z \mid A=a)} for the distinguishing weight
  • _thm-causal-marginal-risk.qmd: \teal{\pi(a, Z)} for the shared term, \red{\p(Z=z)} for the marginal weight

\teal is defined at latex-macros/macros.qmd:216 as \providecommand{\teal}[1]{{\color{teal}{#1}}}. The contrast between teal (shared term) and red (key distinguishing weight) is clear in both light and dark modes.

2. Collapsibility wording in _sec-logistic-marginal-considerations.qmd — ✅

The incorrect "Risk ratios are also called collapsible" claim is gone. The text now correctly states:

  • "Risk differences are collapsible" — proven by linearity of expectation, with the explicit formula
  • "Risk ratios are not collapsible in general either" — supported by $\frac{\E{\pi(1,Z)}}{\E{\pi(0,Z)}} \neq \E{\frac{\pi(1,Z)}{\pi(0,Z)}}$
  • "Odds ratios are non-collapsible in an even stronger sense" — can differ from every conditional OR even with no confounding

All three statements are consistent with #thm-collapsibility. The inequality formula for RRs is mathematically correct (ratio of expectations ≠ expectation of ratios). The progression from "fully collapsible" (RD) → "not collapsible" (RR) → "even stronger non-collapsibility" (OR) is pedagogically clear.

3. Slidebreak structure at _sec-logistic-RR-RD.qmd L80–84 — ✅

The structure is now:

{{< slidebreak >}}       ← L80  (starts a new slide)
### Notes and caveats   ← L82  (heading on the same slide as its content)
{{< include _sec-marginal-notes.qmd >}}   ← L84

The ### Notes and caveats heading and its content now appear together on one slide. The previously-flagged title-only slide is gone.


Positive verification of all other aspects

Math — all correct:

  • #thm-observed-marginal-risk: correctly derives $\pi(a) = \mathbb{E}[\pi(a,Z) \mid A=a]$ via the law of total probability, using $f_{Z \mid A}$ as weights
  • #thm-causal-marginal-risk: correctly derives $\pi_a = \mathbb{E}[\pi(a,Z)]$ under consistency + conditional exchangeability + positivity, using marginal $f_Z$
  • The two theorems together make the observational/causal distinction immediately visible
  • #def-collapsibility + #thm-collapsibility: rigorous, covers both causal ($f_Z$ weights) and observational ($f_{Z \mid A}$ weights) cases
  • #exm-collapsibility: both assumptions (no confounding AND equal stratum sizes) correctly stated; shows RD collapses, RR and OR do not; correctly notes non-collapsibility ≠ effect-measure modification

Slidebreak policy (per updated CLAUDE.md lines):

  • All #def-, #thm-, #exm- divs are preceded by {{< slidebreak >}}
  • When a subfile starts directly with a div (_exm-collapsibility.qmd, _thm-collapsibility.qmd, _thm-observed-marginal-risk.qmd, _thm-causal-marginal-risk.qmd), the slidebreak is in the including file, not in the subfile itself ✅

Heading levels — no jumps:

  • ## Deriving risk ratios…### Computing marginal risk differences#### Conceptual approach / #### Marginal vs. conditional effects / #### Standard errors and CIs### Bootstrap inference (sibling) → ### Computing marginal risk ratios (sibling) → ### Notes and caveats (sibling). All transitions are valid.
  • ##### Fit the model / ##### Compute the point estimate / etc. inside #exm-wcgs-marginal-rd are at the correct depth.

Subfile structure:

  • All new subfiles begin with prose (not a heading), conforming to the CLAUDE.md policy ✅
  • All subfiles are ≤ 108 lines (excluding _sec-marginal-vs-conditional.qmd at 149, explicitly deferred)

R code quality:

  • make_counterfactual_datasets() validates factor levels with an informative error message ✅
  • invisible(load(...)) suppresses stray output ✅
  • B <- 300 labeled "illustrative only; use ≥ 2000 for final CIs" consistently ✅
  • All numerical results use inline R ✅
  • eval: false on _sec-bootstrap-boot-package.qmd and _sec-marginal-rr.qmd keeps render time manageable ✅

Cross-references verified:

  • def-consistency, def-exchangeability, def-positivity_subfiles/causal-inference/
  • sec-bootstrap-cichapters/basic-statistical-methods.qmd
  • def-potential-outcomes_subfiles/causal-inference/

One deferred item (not blocking)

_sec-marginal-vs-conditional.qmd is 149 lines (over the ~100 soft target). The author explicitly deferred splitting it to a future pass. Not blocking.


Status: all previously-blocking issues are resolved. This PR is ready for WIP label removal and merge.

@github-actions

Copy link
Copy Markdown
Contributor

Working tree is clean — no code changes to make (this is a review). Now I'll post my findings.


Code Review — PR #706

All three must-fix items from the 2026-06-18 review are resolved:

1. \blue\teal dark-mode fix ✅ confirmed

Both _thm-observed-marginal-risk.qmd and _thm-causal-marginal-risk.qmd now use \teal{...} in the statement and proof. Color change is consistent across both files.

2. Collapsibility wording ✅ confirmed

_sec-logistic-marginal-considerations.qmd now correctly states:

Risk ratios are not collapsible in general either: the marginal RR is not a weighted average of conditional RRs, because a ratio of expectations is not the same as an expectation of ratios

This is consistent with @thm-collapsibility and the inequality displayed two lines below. The OR non-collapsibility sentence ("even stronger sense…can differ from every conditional OR even when there is no confounding") is also mathematically correct.

3. ### Notes and caveats isolated slide ✅ resolved (author decision)

One {{< slidebreak >}} before the heading, no second one between heading and #def-collapsibility. The heading and definition now share a slide; that's a reasonable design for this section.


Broader checks — all pass

Math: All estimands, theorems, and proofs verified:

  • #thm-observed-marginal-risk: $\pi(a) = \E{\pi(a,Z)\mid A=a}$ via law of total probability ✅
  • #thm-causal-marginal-risk: $\pi_a = \E{\pi(a,Z)}$ under consistency + conditional exchangeability + positivity ✅
  • #def-collapsibility + collapsibility bias definition ✅
  • #thm-collapsibility: additive measures collapsible by linearity; ratio and log-odds measures not ✅
  • #exm-collapsibility: both simplifying assumptions (no confounding + equal stratum sizes) stated and numerically verified ✅

Notation: $\pi(a)$ (observed, $f_{Z\mid A}$ weights) vs. $\pi_a$ (causal, $f_Z$ weights) maintained throughout ✅

Heading levels: ############### chain followed through the include hierarchy (div-title headings ignored per CLAUDE.md) ✅

Slidebreaks: Slidebreak before every #def-, #thm-, #exm- div in each file's including context ✅

R code:

  • g-computation implemented correctly; compute_marginal_rd standardizes over the same bootstrap resample as the model ✅
  • invisible(load(...)) suppresses stray output ✅
  • B <- 300 labeled "illustrative only; use ≥ 2000 for final CIs" ✅
  • make_counterfactual_datasets() validates factor levels ✅

Cross-references: .qmd-relative links (causal-inference.qmd#def-consistency etc.) work in Quarto websites ✅

WORDLIST: "Collapsibility"/"collapsibility" correctly added ✅


One remaining administrative action

Two CHANGES_REQUESTED reviews (both your own, from 2026-05-18 and 2026-05-19) are still open in GitHub's review state. Their inline comments are all addressed in the current branch, but GitHub will still show the PR as blocked by those reviews. You'll need to dismiss them (Settings → Reviews → Dismiss) before the merge button goes green.


Verdict: no new bugs found. The optional file-length item (_sec-marginal-vs-conditional.qmd, 149 lines) remains deferred per your note. This PR looks ready to remove [WIP] from the title once you've dismissed those two self-reviews.

— posted by @claude post-step from workflow run

@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
🚀 Preview available at: https://d-morrison.github.io/rme/pr-preview/pr-706/

Built to branch gh-pages at 2026-06-19 08:28 UTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add a section to logistic regression explaining how to compute marginal risk differences from logistic models

5 participants