Skip to content

revise sec-double-integrals#843

Open
github-actions[bot] wants to merge 24 commits into
mainfrom
claude/issue-841-20260529-172816
Open

revise sec-double-integrals#843
github-actions[bot] wants to merge 24 commits into
mainfrom
claude/issue-841-20260529-172816

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Draft PR opened by @claude to address #841.

Triggered by workflow run.

Addresses #841.

…ordering policy

Rearrange sec-double-integrals so each theorem is immediately followed by
its proof and examples, per the policy: thm → proof → example → counterexample.

New order:
  thm-fubini → exm-fubini-nonrect → cor-fubini-rect + proof →
  exm-fubini-rect → exm-fubini-fail (Riemann part only) →
  thm-fubini-tonelli + notes → exm-fubini-fail-tonelli (new, measure-theoretic part)

Split exm-fubini-fail into two: the Riemann counterexample (kept as
exm-fubini-fail) and a new exm-fubini-fail-tonelli that shows why
thm-fubini-tonelli's absolute-integrability condition is also essential.

Also codify the general ordering policy in CLAUDE.md under Content Writing.

Closes #841

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot requested a review from d-morrison May 29, 2026 17:52
@d-morrison d-morrison marked this pull request as ready for review May 29, 2026 18:48
@d-morrison d-morrison closed this May 29, 2026
@github-actions github-actions Bot removed the request for review from d-morrison May 29, 2026 18:48
@d-morrison d-morrison reopened this May 29, 2026
@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

@github-actions github-actions Bot requested review from d-morrison and removed request for d-morrison May 29, 2026 18:49
@claude

claude Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

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


PR #843 Code Review: revise sec-double-integrals

  • Gather context and read diff
  • Read full changed file for context
  • Review CLAUDE.md changes
  • Review _sec_double_integrals.qmd changes
  • Post findings

Summary

This PR restructures _sec_double_integrals.qmd to comply with a new content-ordering policy, which is simultaneously codified in CLAUDE.md. The central change: move @exm-fubini-nonrect to sit directly after @thm-fubini, move @thm-fubini-tonelli after the failure counterexample, and promote the Fubini–Tonelli failure discussion from a paragraph in @exm-fubini-fail into its own new @exm-fubini-fail-tonelli example. The math is correct, the cross-references all resolve backward, and the reorganization is pedagogically sound.


CLAUDE.md changes

Three new rules added to ### Content Writing:

- After every definition or concept, include a concrete example … if a counterexample is feasible, include one immediately after the example
- After every theoretical claim (thm, cor, lem): (1) immediately follow with a proof, then (2) immediately follow with an example utilizing the claim
- Always arrange divs and sections so that related items are adjacent: theorem → proof → example(s) → counterexample(s)

The rules are clear and sensible. One internal tension:

The new proof rule is not self-consistently applied to @thm-fubini. The Riemann Fubini theorem (@thm-fubini, line 15) has no :::proof div after it — neither in origin/main nor in this PR. The new rule says every thm must immediately be followed by a proof. If the omission of a proof for @thm-fubini is intentional (e.g., the proof is non-elementary and out of scope), the CLAUDE.md rule should acknowledge that exception, or the rule should be softened to "when a proof is feasible/available." Fix this →


_sec_double_integrals.qmd changes

Reordering: correct and well-motivated

Position (new) Div Notes
1 @thm-fubini Unchanged
2 @exm-fubini-nonrect Moved up from after @exm-fubini-rect
3 @cor-fubini-rect + proof Unchanged
4 @exm-fubini-rect Unchanged
5 @exm-fubini-fail Shortened
6 @thm-fubini-tonelli + notes Moved down
7 @exm-fubini-fail-tonelli New

@exm-fubini-nonrect now immediately follows @thm-fubini and correctly cites @thm-fubini Part 1 as justification (line 74). No forward references introduced.

New @exm-fubini-fail-tonelli — correctness check

The polar-coordinates lower bound at _sec_double_integrals.qmd:388–396:

$$\iint_R |f|,dA \ge \int_0^{\pi/2}!\int_0^{\epsilon} \frac{|\cos 2\theta|}{r^2}\cdot r,dr,d\theta = \left(\int_0^{\pi/2}|\cos 2\theta|,d\theta\right)\int_0^\epsilon\frac{dr}{r} = +\infty$$

  • Substitution $x = r\cos\theta,, y = r\sin\theta$: $|f| = r^2|\cos 2\theta|/r^4 = |\cos 2\theta|/r^2$
  • Jacobian $r,dr,d\theta$: inner integrand $|\cos 2\theta|/r^2 \cdot r = |\cos 2\theta|/r$
  • The product factors correctly because $\theta$ and $r$ are independent ✓
  • $\int_0^\epsilon dr/r$ diverges ✓
  • Conclusion "(Hypothesis (a) also fails: $f$ takes both positive and negative values)" ✓

Math is correct.

Issues

1. @thm-fubini-tonelli lacks a positive application example. The new CLAUDE.md rule says to "immediately follow with an example utilizing the claim." The only example that follows @thm-fubini-tonelli is @exm-fubini-fail-tonelli, which shows when the theorem does not apply — a counterexample, not a positive use. The speaker notes explain the theorem's role, but the rule's intent (show the theorem working) is not satisfied. This may be acceptable given that the theorem is out of scope for Epi 204 itself, but it puts the PR out of compliance with the rule it is adding. Fix this →

2. @exm-fubini-fail-tonelli back-references @exm-fubini-fail but has no figure. This is fine — it says "The same function … from @exm-fubini-fail" which correctly points to the figure in the earlier example. No figure duplication needed. ✓

3. Caption update in @fig-fubini-fail (line 319) — changed from mentioning $\iint_R |f|,dA = \infty$ to "f is not continuous on R and @thm-fubini does not apply". This is appropriate since the Tonelli-specific language is now in the new example. ✓

4. @exm-fubini-fail-tonelli (line 407): citation [@wp:fubini] ends without a {{< slidebreak >}} — but this is consistent with how the file ends (original also ended without a trailing slidebreak after the last block). Not an issue.


Verdict

The reorganization is correct and improves content flow. The main concern is the internal consistency gap: the new CLAUDE.md rules require proofs and positive examples after every theorem, but @thm-fubini still has no proof, and @thm-fubini-tonelli only has a failure example. These should either be addressed in the .qmd or the rules should be qualified. Everything else (math, cross-references, R code, div nesting) is correct.

@github-actions github-actions Bot requested a review from d-morrison May 29, 2026 18:52
@github-actions github-actions Bot removed the request for review from d-morrison June 5, 2026 00:49
@claude

claude Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

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


PR #843 Code Review: revise sec-double-integrals

  • Read git diff (origin/main...HEAD)
  • Read full _sec_double_integrals.qmd
  • Verify math correctness
  • Check cross-references and div structure
  • Check CLAUDE.md rule self-consistency
  • Post findings

Summary

This PR restructures _sec_double_integrals.qmd to comply with a new content-ordering policy (simultaneously codified in CLAUDE.md). The central moves:

  • @exm-fubini-nonrect promoted to sit immediately after @thm-fubini
  • @thm-fubini-tonelli demoted to after @exm-fubini-fail
  • Tonelli failure content extracted from @exm-fubini-fail into a new @exm-fubini-fail-tonelli

All math is correct, all cross-references resolve backward, and the reordering is pedagogically sound. The main concern is a self-consistency gap between the new CLAUDE.md rules and the content they govern.


CLAUDE.md changes

Three new rules added to ### Content Writing:

- if a counterexample is feasible, include one immediately after the example
- After every theoretical claim (thm, cor, lem): (1) immediately follow with a proof, then (2) immediately follow with an example utilizing the claim
- Always arrange divs and sections so that related items are adjacent: theorem → proof → example(s) → counterexample(s)

Self-consistency gap — rule requires proofs that are not present:

  • @thm-fubini (line 15): no proof div, immediately followed by @exm-fubini-nonrect. The new rule says every thm must be immediately followed by a proof. A Riemann-form Fubini proof is non-elementary and presumably intentionally omitted. The rule should be qualified (e.g., "when a proof is feasible/in-scope") to avoid requiring proofs for cited external theorems. Fix this →

  • @thm-fubini-tonelli (line 327): no proof div. A proof of the measure-theoretic Fubini–Tonelli requires machinery far beyond the course scope, so the omission is intentional — but again the rule as written requires it.


_sec_double_integrals.qmd changes

Reordering

Position (new) Div Status
1 @thm-fubini Unchanged
2 @exm-fubini-nonrect Moved up from after @exm-fubini-rect
3 @cor-fubini-rect + proof Unchanged
4 @exm-fubini-rect Unchanged
5 @exm-fubini-fail Shortened
6 @thm-fubini-tonelli + notes Moved down
7 @exm-fubini-fail-tonelli New

@exm-fubini-nonrect now cites @thm-fubini Part 1 (line 74) — a backward reference to line 15. No forward references introduced. ✓

@exm-fubini-fail no longer mentions @thm-fubini-tonelli, which is now defined below it — the forward reference is gone. ✓

Math check — new @exm-fubini-fail-tonelli

The polar lower bound for $\iint_R |f|,dA$:

  • Substitution $x=r\cos\theta$, $y=r\sin\theta$: $|f|=|x^2-y^2|/(x^2+y^2)^2 = r^2|\cos2\theta|/r^4 = |\cos2\theta|/r^2$
  • Jacobian $r,dr,d\theta$: inner integrand $|\cos2\theta|/r^2 \cdot r = |\cos2\theta|/r$
  • Fubini factoring over the quarter-disk: $\left(\int_0^{\pi/2}|\cos2\theta|,d\theta\right)!\left(\int_0^\epsilon dr/r\right)$
    (The $\theta$-integral equals 1; the $r$-integral diverges at $r=0$.) ✓
  • "Hypothesis (a) also fails: $f$ takes both positive and negative values" — $f&gt;0$ when $|x|&gt;|y|$, $f&lt;0$ when $|y|&gt;|x|$. ✓

All arithmetic correct.

Issues

1. @thm-fubini-tonelli has no positive-use example (new rule violation).
The new CLAUDE.md rule requires an "example utilizing the claim" immediately after every theorem. The only example following @thm-fubini-tonelli is @exm-fubini-fail-tonelli, which demonstrates a failure case — it shows when neither hypothesis is satisfied and the theorem does not apply. This is a counterexample, not a positive application. To comply with the new rule, a positive example should precede @exm-fubini-fail-tonelli (e.g., a nonneg function on a product space where Tonelli's condition is met). If the theorem's scope is intentionally out-of-range for the course, the CLAUDE.md rule should acknowledge that positive examples are only required when in scope. Fix this →

2. @exm-fubini-fail-tonelli uses :::: (four-colon fence) but contains no nested divs.
Four colons are needed when a div must wrap a nested three-colon div (e.g., a figure inside an example). @exm-fubini-fail-tonelli has no such nesting — ::: (three colons) is sufficient and consistent with @thm-fubini-tonelli, @cor-fubini-rect, etc. Fix this →

3. @cor-fubini-rect and @exm-fubini-rect interrupt the @thm-fubini cluster.
Under the new rule "theorem → proof → example(s) → counterexample(s)", the counterexample for @thm-fubini (@exm-fubini-fail) should be adjacent to @thm-fubini's examples. Currently @cor-fubini-rect + proof + @exm-fubini-rect sit between @exm-fubini-nonrect and @exm-fubini-fail, separating the Fubini positive-example cluster from its counterexample. This is a practical compromise (the corollary naturally follows the theorem) but is technically non-compliant with the adjacency rule. No proposed fix — noting as a known gap for the maintainer to decide whether to tighten the rule or re-order.


Non-issues

  • @fig-fubini-fail caption update (line 319): changed from "Fubini–Tonelli fails" to "not continuous → @thm-fubini doesn't apply" — accurate now that the Tonelli discussion has moved to @exm-fubini-fail-tonelli. ✓
  • :::notes div (lines 360–369) sits between @thm-fubini-tonelli and the next {{< slidebreak >}} — correctly associated with the theorem's slide. ✓
  • @exm-fubini-fail-tonelli back-references @exm-fubini-fail — no figure duplication needed since both examples share the same function. ✓
  • All {{< slidebreak >}} markers are present between major blocks. ✓

Verdict

The reorganization is correct and pedagogically sound. Issues 1 and 3 are the most important: the PR adds rules it cannot fully satisfy. Issue 2 is a minor stylistic inconsistency. Recommend addressing at minimum Issue 1 (either add a positive example for @thm-fubini-tonelli, or qualify the rule) before merging.

@github-actions github-actions Bot requested a review from d-morrison June 5, 2026 00:53
@d-morrison

Copy link
Copy Markdown
Owner

@claude address all review comments

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

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

claude Bot and others added 4 commits June 5, 2026 01:20
…i div fences, add positive Fubini-Tonelli example
…i example

Issue 2: change :::: -> ::: for exm-fubini-fail-tonelli (no nested divs)
Issue 3: add @exm-fubini-tonelli-apply before @exm-fubini-fail-tonelli showing
  f(x,y) = e^{-(x+y)} >= 0 on [0,1]x[0,1] satisfies Tonelli hypothesis (a)
  and both iterated integrals equal (1-e^{-1})^2
@github-actions github-actions Bot removed the request for review from d-morrison June 5, 2026 17:03
@claude

claude Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

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


PR #843 Code Review: revise sec-double-integrals (follow-up)

  • Read git diff (origin/main...HEAD)
  • Read full _sec_double_integrals.qmd current state
  • Verify all previous review issues were addressed
  • Check math correctness of new example
  • Post findings

Summary

All three actionable issues from previous reviews have been addressed. Two minor residual items noted below.


Previous issues — resolution status

# Issue Status
1 CLAUDE.md proof rule too strict (required proofs for all theorems including cited external ones) Fixed
2 @thm-fubini-tonelli had no positive-application example Fixed
3 @exm-fubini-fail-tonelli used :::: (four colons) without nested divs Fixed
4 @cor-fubini-rect cluster interrupts the @thm-fubini adjacency (intentional gap — still present, no fix required)

Detail

Issue 1 — CLAUDE.md proof rule

The rule now reads:

After every theoretical claim (thm, cor, lem): when a proof is feasible and in scope, (1) immediately follow with a proof, then (2) immediately follow with an example utilizing the claim; when a proof is non-elementary or out of scope (e.g., @thm-fubini-tonelli), omit the proof block and proceed directly to the example.

This is correct. The exception criterion ("non-elementary or out of scope") is general enough to cover @thm-fubini as well, even though @thm-fubini is not explicitly named. Minor note: the e.g. only names @thm-fubini-tonelli, which could prompt a future reader to wonder why @thm-fubini also lacks a proof. Not a blocker — the rule's criterion covers it implicitly.

Issue 2 — New positive example @exm-fubini-tonelli-apply

Math check:

$f(x, y) = e^{-(x+y)}$ on $[0,1]^2$ with Lebesgue measure.

  • Hypothesis (a) (Tonelli, nonnegativity): $e^{-(x+y)} \ge 0$
  • Inner integral: $\int_0^1 e^{-(x+y)},dy = e^{-x}(1 - e^{-1})$
  • Outer integral: $(1 - e^{-1})\int_0^1 e^{-x},dx = (1 - e^{-1})^2$
  • Product form claim: $\left(\int_0^1 e^{-x},dx\right)^2 = (1-e^{-1})^2$ since $f$ factors ✓
  • Conclusion "both iterated integrals agree, as @thm-fubini-tonelli guarantees" ✓

All arithmetic correct.

Issue 3 — Fence fix

@exm-fubini-fail-tonelli now opens with :::{#exm-fubini-fail-tonelli} (three colons) and closes with :::. ✓


Remaining minor issues

1. Missing blank line before {{< slidebreak >}} at the end of @exm-fubini-tonelli-apply.
The closing ::: of @exm-fubini-tonelli-apply is immediately followed by {{< slidebreak >}} without an intervening blank line (lines 396–397 of current file). Every other slidebreak in the file has a blank line before it (e.g., after :::: at lines 123–125). This is a style inconsistency; it is unlikely to affect rendering but should be made uniform for readability.
Fix this →

2. CLAUDE.md exception example names only @thm-fubini-tonelli, not @thm-fubini.
The new rule's parenthetical exception reads "e.g., @thm-fubini-tonelli" — but @thm-fubini (Riemann form) also has no proof and also satisfies the "non-elementary or out of scope" criterion. A future contributor could reasonably wonder why @thm-fubini has no proof while the rule's only named example is the other theorem. Low priority: the general criterion covers it, but adding @thm-fubini to the example list would make the intent unambiguous.
Fix this →


Verdict

All three prior actionable findings are resolved. The two remaining items are low-priority style/clarity nits and do not block merging. The math in @exm-fubini-tonelli-apply is correct, the div fences are consistent, and the CLAUDE.md rule is appropriately qualified.

@github-actions github-actions Bot requested a review from d-morrison June 5, 2026 17:06
@d-morrison

Copy link
Copy Markdown
Owner

@claude address all review comments, even nits

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

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

@d-morrison

Copy link
Copy Markdown
Owner

@claude can we switch to examples involving probabilities?

@github-actions github-actions Bot requested a review from d-morrison June 9, 2026 20:28
@github-actions github-actions Bot removed the request for review from d-morrison June 14, 2026 03:31
@claude

claude Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

PR #843 Code Review: revise sec-double-integrals

  • Read git diff (origin/main...HEAD)
  • Read full _sec_double_integrals.qmd
  • Review CLAUDE.md committed state
  • Verify math correctness and cross-references
  • Post findings as inline comments

View job run

Reword the example's intro sentence to say the case is one where
neither Fubini-Tonelli condition is satisfied, matching the title
"When neither Fubini-Tonelli condition is satisfied".
@github-actions github-actions Bot requested review from d-morrison and removed request for d-morrison June 14, 2026 03:32
@claude

claude Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

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


PR #843 Code Review: revise sec-double-integrals

  • Read git diff (origin/main...HEAD)
  • Read full _sec_double_integrals.qmd
  • Review CLAUDE.md committed state
  • Verify math correctness and cross-references
  • Post findings

Summary

The latest commit (cb642ab) fixes the last open nit from the prior review round: the intro sentence of @exm-fubini-fail-tonelli now reads "illustrates a case where neither condition of @thm-fubini-tonelli is satisfied," matching the updated example title "When neither Fubini–Tonelli condition is satisfied."

All prior actionable findings are resolved. This PR is ready to merge.


Prior review issues — final status

Issue Status
Proof rule too strict for cited external theorems ✓ Fixed — rule names both @thm-fubini and @thm-fubini-tonelli as exceptions
@thm-fubini-tonelli had no positive example ✓ Fixed — @exm-fubini-tonelli-apply added (Exp(1) probability framing)
@exm-fubini-fail-tonelli used :::: without nested divs ✓ Fixed — now :::
Missing blank line before slidebreak after @exm-fubini-tonelli-apply ✓ Fixed
CLAUDE.md exception only named @thm-fubini-tonelli ✓ Fixed — now names both
@exm-fubini-rect "Adapted from" citation misleading ✓ Fixed — "Structure adapted from…; integrand is original"
Missing trailing newline at EOF ✓ Fixed
@exm-fubini-fail-tonelli title mentioned only hypothesis (b) ✓ Fixed — "When neither Fubini–Tonelli condition is satisfied"
@exm-fubini-rect second iterated integral lacked intermediate steps ✓ Fixed — full steps now shown for both orders
@exm-fubini-fail-tonelli intro mentioned only hypothesis (b) Fixed in cb642ab
M CLAUDE.md in working tree CI restore artifact (tracked in #891) — not actionable; confirmed git show HEAD:CLAUDE.md contains all 3 new Content Writing rules

CLAUDE.md — committed HEAD is correct

All three new Content Writing rules are in the committed branch HEAD (verified: git show HEAD:CLAUDE.md | grep -c '...' = 3):

  • if a counterexample is feasible, include one immediately after the example
  • After every theoretical claim (thm, cor, lem): when a proof is feasible and in scope…; when a proof is non-elementary or out of scope (e.g., @thm-fubini, @thm-fubini-tonelli), omit the proof block and proceed directly to the example
  • Always arrange divs and sections so that related items are adjacent: theorem → proof → example(s) → counterexample(s)

Math verification (full file)

@exm-fubini-nonrect$\mathbb{E}[e^{-X^2}\mathbf{1}(Y \le X)]$ for $X, Y \sim \text{Uniform}(0,1)$

  • $\iint_D e^{-x^2},dA$ where $D = {0 \le y \le x \le 1}$
  • Inner ($dy,dx$): $\int_0^x dy = x$ → outer: $\int_0^1 x e^{-x^2},dx = [-\tfrac{1}{2}e^{-x^2}]_0^1 = (e-1)/(2e)$

@exm-fubini-rect$\mathbb{E}[X^2 + Y^2]$ for $X, Y \sim \text{Uniform}(0,1)$

  • $y$-first: $\int_0^1[x^2y + y^3/3]_0^1,dx = \int_0^1(x^2+\tfrac{1}{3}),dx = [x^3/3+x/3]_0^1 = \tfrac{2}{3}$
  • $x$-first: $\int_0^1[x^3/3+y^2x]_0^1,dy = \int_0^1(\tfrac{1}{3}+y^2),dy = [y/3+y^3/3]_0^1 = \tfrac{2}{3}$
  • Linearity cross-check: $\mathbb{E}[X^2]+\mathbb{E}[Y^2] = \tfrac{1}{3}+\tfrac{1}{3} = \tfrac{2}{3}$

@exm-fubini-fail$f = (x^2-y^2)/(x^2+y^2)^2$

  • $\partial_y[y/(x^2+y^2)] = (x^2-y^2)/(x^2+y^2)^2$ ✓; $y$-first gives $\pi/4$
  • $\partial_x[-x/(x^2+y^2)] = (x^2-y^2)/(x^2+y^2)^2$ ✓; $x$-first gives $-\pi/4$

@exm-fubini-tonelli-apply$X, Y \sim \text{Exp}(1)$, hypothesis (a) satisfied

  • $P(X \le 1, Y \le 1) = (1-e^{-1})^2$ ✓; factoring $(e^{-x}\cdot e^{-y})$ confirms ✓

@exm-fubini-fail-tonelli — polar lower bound

  • $|f| = |\cos 2\theta|/r^2$; Jacobian $r$; inner integrand $|\cos 2\theta|/r$
  • $\int_0^{\pi/2}|\cos 2\theta|,d\theta = \tfrac{1}{2} + \tfrac{1}{2} = 1$
  • $\int_0^\epsilon dr/r$ diverges → product $= +\infty$
  • Intro now says "neither condition of @thm-fubini-tonelli is satisfied" ✓ (matches title)

Content structure and div fences

Position Div Fence Notes
1 @thm-fubini ::: No proof — external cited thm, exception applies ✓
2 @exm-fubini-nonrect :::: Contains :::{#fig-fubini-nonrect}
3 @cor-fubini-rect + proof :::
4 @exm-fubini-rect :::: Contains :::{#fig-fubini-rect}
5 @exm-fubini-fail :::: Contains :::{#fig-fubini-fail}
6 @thm-fubini-tonelli + :::notes ::: No proof — exception applies ✓
7 @exm-fubini-tonelli-apply ::: No nested divs — 3 colons correct ✓
8 @exm-fubini-fail-tonelli ::: No nested divs — 3 colons correct ✓

All {{< slidebreak >}} markers have blank lines before them ✓
All cross-references resolve backward; no forward references ✓
File ends with trailing newline ✓


Verdict

No blocking issues. All prior findings are resolved and the last open nit is fixed in cb642ab. Ready to merge.

@github-actions github-actions Bot requested a review from d-morrison June 14, 2026 03:36

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this file is too long; decompose. note in ai instructions: source files should be under ~100 lines if possible

@dem-extra1

Copy link
Copy Markdown

@claude see review

@github-actions

Copy link
Copy Markdown
Contributor Author

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

@github-actions github-actions Bot removed the request for review from d-morrison June 16, 2026 06:06
@claude

claude Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

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


PR #843 Code Review: revise sec-double-integrals

  • Gather context and read PR history
  • Read full _sec_double_integrals.qmd
  • Review CLAUDE.md committed state
  • Analyze file structure for decomposition
  • Post findings

Summary

The review by @dem-extra1 requests two things: (1) decompose _subfiles/math-prereqs/_sec_double_integrals.qmd (currently 462 lines) into smaller subfiles under ~100 lines each, and (2) add a size convention to CLAUDE.md. Specific decomposition plan and CLAUDE.md fix described below.


Issue 1 — _sec_double_integrals.qmd is 462 lines (CHANGES REQUESTED)

The file contains 8 logically independent blocks. Each can become its own subfile. The {{< include >}} in chapters/math-prereqs.qmd line 51 would expand to 7 sequential includes.

Proposed files in _subfiles/math-prereqs/ (replacing _sec_double_integrals.qmd):

File Content Approx. lines
_fubini_intro_thm.qmd Intro prose (lines 1–12) + @thm-fubini (lines 15–50) ~50
_exm_fubini_nonrect.qmd @exm-fubini-nonrect (lines 52–127) ~76
_cor_fubini_rect.qmd @cor-fubini-rect + proof (lines 129–158) ~30
_exm_fubini_rect.qmd @exm-fubini-rect (lines 160–241) ~82
_exm_fubini_fail.qmd @exm-fubini-fail (lines 243–343) ~101
_thm_fubini_tonelli.qmd @thm-fubini-tonelli + notes + @exm-fubini-tonelli-apply (lines 345–424) ~80
_exm_fubini_fail_tonelli.qmd @exm-fubini-fail-tonelli (lines 426–462) ~37

Every file is ≤101 lines. _exm_fubini_fail.qmd barely exceeds 100 (it contains a figure with ~30 lines of R code); if stricter compliance is desired, the figure chunk can be extracted as _fig_fubini_fail.qmd and included within, but this seems unnecessary given the "if possible" qualifier.

Change to chapters/math-prereqs.qmd (lines 49–51):

Replace:

# Double Integrals {#sec-double-integrals}

{{< include _subfiles/math-prereqs/_sec_double_integrals.qmd >}}

With:

# Double Integrals {#sec-double-integrals}

{{< include _subfiles/math-prereqs/_fubini_intro_thm.qmd >}}

{{< include _subfiles/math-prereqs/_exm_fubini_nonrect.qmd >}}

{{< include _subfiles/math-prereqs/_cor_fubini_rect.qmd >}}

{{< include _subfiles/math-prereqs/_exm_fubini_rect.qmd >}}

{{< include _subfiles/math-prereqs/_exm_fubini_fail.qmd >}}

{{< include _subfiles/math-prereqs/_thm_fubini_tonelli.qmd >}}

{{< include _subfiles/math-prereqs/_exm_fubini_fail_tonelli.qmd >}}

Fix this →


Issue 2 — CLAUDE.md needs a file-length convention (CHANGES REQUESTED)

The ### File Structure section in CLAUDE.md has no guidance on file length. Add one line:

In CLAUDE.md, after the line - Link to .qmdsource files, not rendered.html files (line 38), add:

- Aim to keep source files under ~100 lines; split longer files into named subfiles

Fix this →


Non-issues

  • The math and content in the file are correct (verified in prior review rounds) — decomposition is purely structural.
  • Splitting into subfiles does not affect cross-references: Quarto resolves @exm-fubini-nonrect, @thm-fubini, etc. document-wide regardless of which subfile defines them.
  • The clear freezer label is already on this PR; it will still be needed after the decomposition since _subfiles/ are being modified.

Verdict

Two changes needed before merge: (1) decompose the 462-line file into 7 subfiles ≤101 lines each, and (2) add the ~100-line size convention to CLAUDE.md ### File Structure.

@github-actions github-actions Bot requested a review from d-morrison June 16, 2026 06:08
Per review on #843: split the 462-line _sec_double_integrals.qmd into
seven focused subfiles (each <=99 lines), one per logical block:

- _fubini_intro_thm.qmd      (intro + @thm-fubini)
- _exm_fubini_nonrect.qmd    (@exm-fubini-nonrect)
- _cor_fubini_rect.qmd       (@cor-fubini-rect + proof)
- _exm_fubini_rect.qmd       (@exm-fubini-rect)
- _exm_fubini_fail.qmd       (@exm-fubini-fail)
- _thm_fubini_tonelli.qmd    (@thm-fubini-tonelli + notes + @exm-fubini-tonelli-apply)
- _exm_fubini_fail_tonelli.qmd (@exm-fubini-fail-tonelli)

The chapters/math-prereqs.qmd include now expands to seven sequential
includes. The slidebreaks that separated the blocks move into the
parent between includes (per the convention that the initial slidebreak
before a thm-variant div belongs in the including file); internal
slidebreaks (intro->thm, tonelli->apply) stay within their subfile.
Content is byte-identical to the original; no math or cross-references
changed.

https://claude.ai/code/session_01NQSLJmjMN22RpwpgJ6Lw6u
@github-actions github-actions Bot removed the request for review from d-morrison June 16, 2026 06:15
@claude

claude Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

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


PR #843 Code Review: revise sec-double-integrals

  • Read git diff (origin/main...HEAD)
  • Read all 7 new subfiles
  • Verify CLAUDE.md file-length convention added
  • Check chapters/math-prereqs.qmd include directives
  • Verify content preserved correctly
  • Check cross-references and file sizes
  • Post findings

Summary

The latest commit (9672218) decomposes _subfiles/math-prereqs/_sec_double_integrals.qmd into 7 subfiles as requested by @dem-extra1. The decomposition is structurally correct — all content is preserved, slidebreaks are placed correctly, and every subfile is ≤ 100 lines. One omission: the CLAUDE.md ### File Structure section was not updated with the requested file-length convention.


Decomposition — structural review

Subfile Content Lines
_fubini_intro_thm.qmd Intro prose + {{< slidebreak >}} + @thm-fubini 47
_exm_fubini_nonrect.qmd @exm-fubini-nonrect 74
_cor_fubini_rect.qmd @cor-fubini-rect + proof 28
_exm_fubini_rect.qmd @exm-fubini-rect 80
_exm_fubini_fail.qmd @exm-fubini-fail 99
_thm_fubini_tonelli.qmd @thm-fubini-tonelli + notes + {{< slidebreak >}} + @exm-fubini-tonelli-apply 78
_exm_fubini_fail_tonelli.qmd @exm-fubini-fail-tonelli 37

All subfiles are ≤ 100 lines ✓. No subfile begins with a section heading ✓.

Slidebreakschapters/math-prereqs.qmd inserts one {{< slidebreak >}} between each pair of consecutive includes. This matches the original inter-block slidebreaks exactly:

  • Internal slidebreaks within _fubini_intro_thm.qmd (between intro prose and @thm-fubini) and within _thm_fubini_tonelli.qmd (between notes and @exm-fubini-tonelli-apply) are preserved in the subfiles ✓
  • The six parent-level slidebreaks cover the six inter-block gaps that were in the original file ✓
  • No duplicate or missing slidebreaks ✓

Cross-references — Quarto resolves cross-references document-wide regardless of include boundaries. All @exm-..., @thm-..., @cor-..., @fig-... labels are identical to those in the original file and resolve backward ✓

Div fence depths — all :::: (four colons) for divs containing a nested ::: figure, and ::: everywhere else — unchanged from the reviewed version ✓


Issue — CLAUDE.md file-length convention missing

@dem-extra1's review comment explicitly requested: "note in ai instructions: source files should be under ~100 lines if possible." The committed HEAD:CLAUDE.md ### File Structure section currently reads:

### File Structure
- Subfiles (`_subfiles/`) must NOT begin with a section heading — place headings in the parent `.qmd`
- Link to `.qmd` source files, not rendered `.html` files
- `_extensions/` is vendored third-party code — do not review or modify it

No file-length guideline was added. The decomposition itself demonstrates the convention, but the instruction is absent from CLAUDE.md. Fix this →


Non-issues

  • Content (math, prose, figures, R code) is identical to the previously-reviewed version; all math was verified correct in prior rounds ✓
  • chapters/math-prereqs.qmd: single {{< include ... >}} replaced with 7 sequential includes, each preceded by a blank {{< slidebreak >}} separator (except the first include, which immediately follows the section heading) ✓
  • clear freezer label already present on the PR ✓
  • M CLAUDE.md in working tree: CI restore artifact (tracked in #891) — not actionable ✓

Verdict

The decomposition is correct and complete. One item remains from @dem-extra1's review: add the file-length guideline to CLAUDE.md ### File Structure. Once that line is added, the PR is ready to merge.

Copy link
Copy Markdown
Owner

Addressed the review:

  • Issue 1 (decompose the 462-line file) — done here in 96722186c. _sec_double_integrals.qmd is split into 7 subfiles (each ≤99 lines): _fubini_intro_thm, _exm_fubini_nonrect, _cor_fubini_rect, _exm_fubini_rect, _exm_fubini_fail, _thm_fubini_tonelli, _exm_fubini_fail_tonelli. The chapters/math-prereqs.qmd include now expands to 7 sequential includes; the block-separating slidebreaks moved into the parent (per the "initial slidebreak before a thm-variant div belongs in the including file" convention), with internal slidebreaks kept in their subfile. Content is byte-identical; lint clean; all 7 includes resolve.
  • Issue 2 (CLAUDE.md ~100-line convention) — split into its own dedicated PR, CLAUDE.md: add ~100-line source file length convention #910, per the CLAUDE.md rule that meta/infra changes go in a separate PR rather than mixing with book content. So it intentionally does not appear in this PR's diff.

Generated by Claude Code

@github-actions github-actions Bot requested a review from d-morrison June 16, 2026 06:19
@github-actions

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

Built to branch gh-pages at 2026-06-16 06:34 UTC.

d-morrison added a commit to palakgupta29/rme that referenced this pull request Jun 16, 2026
claude-code-action allows file edits by default and `claude_args
--allowed-tools` is additive, but a session that inspects its own
allowlist and sees only Bash/gh/git/WebFetch entries can wrongly
conclude editing is disallowed and post its diff as a comment instead
of committing (PR d-morrison#843 run 27032044840). List Edit/Write/MultiEdit
explicitly so the grant is unambiguous in the string the agent sees.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
d-morrison pushed a commit to palakgupta29/rme that referenced this pull request Jun 16, 2026
Adds a File Structure guideline to keep source files under ~100 lines
and split longer files into named subfiles, matching the decomposition
practice requested in review and applied in PR d-morrison#843.

Kept as its own dedicated PR per the CLAUDE.md rule that meta/infra
changes don't mix with book-content PRs.

https://claude.ai/code/session_01NQSLJmjMN22RpwpgJ6Lw6u
@github-actions github-actions Bot removed the request for review from d-morrison June 18, 2026 17:01
@claude

claude Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

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


PR #843 Code Review: revise sec-double-integrals

  • Read git diff (origin/main...HEAD)
  • Read all 7 new subfiles and chapters/math-prereqs.qmd
  • Review CLAUDE.md committed state
  • Verify content preservation and cross-references
  • Check div fence depths and slidebreak placement
  • Post findings

Summary

The latest commit (9672218) decomposes _subfiles/math-prereqs/_sec_double_integrals.qmd into 7 subfiles per @dem-extra1's review request. The decomposition is structurally correct, all prior content-review findings remain resolved, and no new issues are found. This PR is ready to merge.


Decomposition review

Subfile Content Lines
_fubini_intro_thm.qmd Intro prose + {{< slidebreak >}} + @thm-fubini 47
_exm_fubini_nonrect.qmd @exm-fubini-nonrect (with figure) 74
_cor_fubini_rect.qmd @cor-fubini-rect + proof 28
_exm_fubini_rect.qmd @exm-fubini-rect (with figure) 80
_exm_fubini_fail.qmd @exm-fubini-fail (with figure) 99
_thm_fubini_tonelli.qmd @thm-fubini-tonelli + notes + {{< slidebreak >}} + @exm-fubini-tonelli-apply 78
_exm_fubini_fail_tonelli.qmd @exm-fubini-fail-tonelli 37

All 7 subfiles are ≤99 lines — within the ~100-line guideline now codified in CLAUDE.md line 39 (already present on origin/main via the merged PR #910). ✓

No subfile begins with a section heading. All subfiles open with either prose or a fenced div (:::, ::::) — section headings inside divs (###, ####) are div titles, not standalone sections. ✓

Slidebreaks.

  • Internal slidebreaks (within a logical unit) remain in the subfile: _fubini_intro_thm.qmd line 12 (between intro prose and @thm-fubini), _thm_fubini_tonelli.qmd line 45 (between the theorem and its positive-application example).
  • Inter-subfile breaks are in the parent chapters/math-prereqs.qmd, with one {{< slidebreak >}} between each pair of consecutive includes.
  • No missing or duplicate slidebreaks. ✓

Div fence depths.

  • :::: (four colons) used at @exm-fubini-nonrect, @exm-fubini-rect, and @exm-fubini-fail — each contains a nested :::{#fig-...} div. ✓
  • ::: (three colons) everywhere else. ✓

Cross-references. All references resolve backward:

  • _exm_fubini_nonrect.qmd@thm-fubini (defined in file 1) ✓
  • _cor_fubini_rect.qmd@thm-fubini
  • _exm_fubini_rect.qmd@cor-fubini-rect (defined in file 3) ✓
  • _exm_fubini_fail.qmd@thm-fubini, @cor-fubini-rect (no forward reference to @thm-fubini-tonelli) ✓
  • _exm_fubini_fail_tonelli.qmd@exm-fubini-fail (file 5), @thm-fubini-tonelli (file 6) ✓

CLAUDE.md — committed state

All three new Content Writing rules are in the committed branch HEAD:

  • if a counterexample is feasible, include one immediately after the example
  • After every theoretical claim (thm, cor, lem): when a proof is feasible and in scope…; when a proof is non-elementary or out of scope (e.g., @thm-fubini, @thm-fubini-tonelli), omit the proof block
  • Always arrange divs and sections so that related items are adjacent: theorem → proof → example(s) → counterexample(s)

The M CLAUDE.md working-tree artifact is the CI restore generated by anthropics/claude-code-action's restoreConfigFromBase() on every job run — not an uncommitted change on the branch. (Tracked in #891.) ✓


Prior review issues — final status

Issue Status
Proof rule too strict for cited external theorems ✓ Fixed
@thm-fubini-tonelli had no positive example ✓ Fixed — @exm-fubini-tonelli-apply (Exp(1) framing)
@exm-fubini-fail-tonelli used :::: without nested divs ✓ Fixed — now :::
Missing blank line before slidebreak after @exm-fubini-tonelli-apply ✓ Fixed
CLAUDE.md exception only named @thm-fubini-tonelli ✓ Fixed — now names both
@exm-fubini-rect "Adapted from" citation misleading ✓ Fixed
Missing trailing newline at EOF ✓ Fixed
@exm-fubini-fail-tonelli title mentioned only hypothesis (b) ✓ Fixed
@exm-fubini-rect second iterated integral lacked intermediate steps ✓ Fixed
@exm-fubini-fail-tonelli intro mentioned only hypothesis (b) ✓ Fixed (cb642ab)
Decompose 462-line file into subfiles ✓ Fixed (9672218)
CLAUDE.md file-length convention ✓ Present (via merged PR #910)
M CLAUDE.md in working tree CI restore artifact — not actionable

Verdict

No blocking issues. The decomposition is correct, all content and slidebreaks are preserved, and all prior findings are resolved. Ready to merge.

@github-actions github-actions Bot requested a review from d-morrison June 18, 2026 17:05
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.

3 participants