Skip to content

Add a figure to the convex intersection example - #665

Merged
d-morrison merged 2 commits into
mainfrom
claude/mds646-convex-intersection-fig
Nov 6, 2026
Merged

d-morrison merged 2 commits into
mainfrom
claude/mds646-convex-intersection-fig

Conversation

@d-morrison

Copy link
Copy Markdown
Contributor

Requested by Ezra · project thread

Adds a figure to exm-convex-intersection: the unit disk's outline, the closed half-disk (its intersection with the half-plane x1 ≥ 0) shaded, and the half-plane's edge. R (ggplot2) and Python (matplotlib) tabs, nested in and cited from the example.

Part of #646.

Files:

  • _subfiles/vector-calc/_fig-convex-intersection.qmd (new)
  • _subfiles/vector-calc/_exm-convex-intersection.qmd

Rendered optimization-convexity.qmd locally and checked the R figure.


Generated by Claude Code

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xfc9pBmXswEX8Uu9Kt7f7
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xfc9pBmXswEX8Uu9Kt7f7
@d-morrison d-morrison self-assigned this Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished review — View run


The PR adds a convex-intersection figure with R and Python tabs and includes it in _exm-convex-intersection.qmd. I found no blocking issues. I did not render the page or run any build.

Checks that passed

  • Cross-reference targets: #def-half-space, #thm-convex-intersection and #def-unit-ball all exist.
  • Figure conventions: the new fig file follows _fig-convex-set.qmd, which sits beside it. That means a panel-tabset with group="language", fig-alt on both chunks and a trailing caption paragraph.
  • Include style: the {{< include mds/_subfiles/... >}} path matches how _fig-convex-set.qmd is included in _sec_optimization_convexity.qmd.
  • R code: it uses ggplot2:: and tibble:: prefixes and the native |> pipe.
  • Macros: the new lines add no math beyond $x_1 \ge 0$ in the caption. That is a coordinate component, not a symbol that has a semantic macro.

Suggestions (non-blocking)

  • _subfiles/vector-calc/_fig-convex-intersection.qmd:90: the caption says "of @exm-convex-intersection", but the figure is included inside that example. The self-reference adds nothing and can go.
  • _subfiles/vector-calc/_exm-convex-intersection.qmd:14: the figure is nested inside the exm- div, whereas fig-convex-set is included at section level. No rule forbids a figure inside an example, only a theorem-type div inside another. The cross-reference should still render. It is worth a glance in the rendered page that the numbering and caption look right.
  • _subfiles/vector-calc/_fig-convex-intersection.qmd:85: the Python chunk ends with plt.show();. The sibling _fig-convex-set.qmd ends with plt.show(). The trailing semicolon is harmless but inconsistent.
  • _subfiles/vector-calc/_fig-convex-intersection.qmd:35-45: the R variable names use an ci_..._ci pattern with a suffix on both ends, which reads oddly. They are also split into many single-use intermediates. This matches the no-nested-calls style, so I'm not asking for a change.

Verdict

Ready for merge

Structured Review Data (JSON)
{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "8563dde7c63bb7e090cde18adb9ed02f5d9b4ac1",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "Checked the new fig subfile and the example include: cross-reference ids, include path, tabset structure and alt text all resolve and match the sibling convex-set figure; only minor stylistic suggestions remain.",
  "holistic_assessment": "The change is scoped to one figure plus its include, follows existing figure conventions, adds no new math macros issues, and carries no build, CI or citation regression risk."
}

I fetched PR #665 live just now. It is open, not a draft and not merged. Its head is 8563dde7c63bb7e090cde18adb9ed02f5d9b4ac1, the commit I reviewed, so the review and its verdict still stand.

Verdict

Ready for merge

Structured Review Data (JSON)
{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "8563dde7c63bb7e090cde18adb9ed02f5d9b4ac1",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "Checked the new fig subfile and the example include: cross-reference ids, include path, tabset structure and alt text all resolve and match the sibling convex-set figure; only minor stylistic suggestions remain.",
  "holistic_assessment": "The change is scoped to one figure plus its include, follows existing figure conventions, adds no new math macros issues, and carries no build, CI or citation regression risk."
}

Reviewed commit: 8563dde

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

💰 Cost: $0.2272 (review) — run

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.2
🚀 Preview available at: https://Morrison-Lab.github.io/mds/pr-preview/pr-665/

Built to branch gh-pages at 2026-11-06 01:24 UTC.

Copy link
Copy Markdown
Contributor Author

Annotations reviewed: 8563dde

All are platform notices (the ubuntu-latest migration, an empty SUBMODULES_TOKEN, a diff-highlight cap); none concern this change.


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


Generated by Claude Code

@d-morrison
d-morrison merged commit 413c23e into main Nov 6, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants