Skip to content

render-pipeline: \frac is an Ord atom, not Inner (ps-calculus +1.82 bp quad/text gap) - #622

Merged
GoKubar merged 1 commit into
mainfrom
agent/daniel-parent/display-frac-quad-gap
Sep 15, 2026
Merged

GoKubar merged 1 commit into
mainfrom
agent/daniel-parent/display-frac-quad-gap

Conversation

@d-q222

@d-q222 d-q222 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

What

\frac is now converted to an Ord atom. It used to be Inner.

In crates/render-pipeline/src/typeset.rs, N::Fraction went through math-layout's Atom::frac, which gives the Inner atom of a bare \over. The pipeline now sets that atom's class to Ord. math-layout and vendor/ are unchanged.

Why

The regression bisect (#609 notes, "Corpus sweep 2 regressions") found a +1.81 bp excess in the \frac{2}{5} \quad \text{as } gap on ps-calculus p1. It is also on main.

Root cause: fraction atom class, not the quad, \text, \nulldelimiterspace or italic correction.

  • latex.ltx 15742 and amsmath.sty 233 both define \frac as {\begingroup#1\endgroup\over#2}. The outer braces make it an Ord atom.
  • As Inner, the fraction got TeX's Inner–Ord thin space (3mu = 1.818 bp at 11 pt) before any following Ord.
  • \dfrac goes through \genfrac, which was already Ord, and was already exact. That made it the discriminating control.
  • A bare \over fills its whole group, so it has no neighbours to space against, and Ord is also right for it.

Minimised against pdflatex (TeX Live, 11 pt article, T1, amsmath). Distances are from the numerator a's origin to as's origin, in bp.

Source pdflatex main this PR
\[ \frac{a}{b} \quad \text{as } x\to 0 \] 17.810 19.627 17.810
\[ \frac{a}{b} \qquad \text{as } … \] 28.656 30.475 28.656
\[ \frac{a}{b} \, \text{as } … \] 8.780 10.598 8.780
\[ \frac{a}{b}\text{as } … \] 6.962 8.780 6.962
Text $\frac{a}{b} \quad \text{as } x$ end. 16.540 18.359 16.540
\[ \dfrac{a}{b} \quad \text{as } … \] (control) 17.810 17.810 17.810

The same +1.818 bp shows with no \quad and no \text at all (\[ \frac{a}{b} x\to 0 \]). y \quad \text{as } was already exact on main.

Real-world ps-calculus, words aligned against reference.pdf within 0.5 bp:

Page main this PR
p1 212/247 222/247
p2 193/244 223/244
p3 318/328 324/328

On p1, as is now at dx −0.807. The \frac{2}{5}as gap is exact (0.000). The remaining offset is the display re-centring from main's 1.61 bp-short \longrightarrow glyph, which #536 fixes.

Overlap check

Test results

  • cargo test --release --test frac_ord_spacing: test result: ok. 1 passed; 0 failed. Without the fix it fails, e.g. \[ \frac{a}{b} \quad \text{as } x\to 0 \]: a -> as 19.627 bp, pdflatex has 17.810.
  • render-pipeline cargo test --release --no-fail-fast: every test binary ok, 376 passed, 0 failed.
  • math-layout cargo test --release: 66 passed, 0 failed.
  • amsmath oracle: TOTAL 59/59 within 0.5 bp
  • amssymb oracle: TOTAL 38/39 within 0.5 bp (32-braces-narrow, same on main)
  • display-placement oracle: TOTAL 33/36 within 0.5 bp (15/17/20, the same three as main)

Not done

  • The compiler doc comment on Nucleus::GenFraction still says "unlike the plain \frac's inner Nucleus::Fraction". It is in crates/compiler / vendor/compiler, so it is left for a compiler PR.
  • math-layout's Atom::frac stays Inner, which is correct for a primitive \over.
  • rank.py pixel scores were not rerun (no flashtex-pdf-exact build in this lane).

(daniel-parent subagent, GH-DISPLAY-FRAC-QUAD-GAP)

🤖 Generated with Claude Code

https://claude.ai/code/session_012c9XLkHjePPGBuarrmE2mz

…xt gap)

What changed:
- `N::Fraction` (`\frac`) converted through math-layout's `Atom::frac`,
  which is the Inner atom of a bare `\over`. latex.ltx 15742 and
  amsmath.sty 233 define `\frac` as `{\begingroup#1\endgroup\over#2}`, so
  the braces make it Ord. As Inner it took TeX's Inner-Ord thin space
  (3mu, 1.818 bp at 11 pt) before any following Ord: the +1.81 bp
  `\frac{2}{5} \quad \text{as }` gap in ps-calculus p1. The pipeline now
  sets the atom's class to Ord. `\dfrac` (`\genfrac`) was already Ord.
- tests/frac_ord_spacing.rs pins pdflatex's numerator-to-`as` distances
  for `\quad`, `\qquad`, `\,`, nothing, inline `\frac`, and `\dfrac`.

What was run:
- cargo test --release --test frac_ord_spacing: `test result: ok. 1 passed`
  (without the fix: 5 cases 1.818 bp too wide, e.g. `a -> as 19.627 bp,
  pdflatex has 17.810`)
- amsmath oracle: `TOTAL 59/59 within 0.5 bp`
- amssymb oracle: `TOTAL 38/39 within 0.5 bp` (32-braces-narrow, same on main)
- display-placement oracle: `TOTAL 33/36 within 0.5 bp` (same 3 as main)

Next step: full render-pipeline suite, PR.

Implementation-Agent: claude-opus-5 subagent of daniel-parent
Commit-Executor: daniel-parent subagent
Claude-Session: https://claude.ai/code/session_012c9XLkHjePPGBuarrmE2mz
@d-q222

d-q222 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

daniel-parent independent review (subagent)

Verdict: APPROVE

Independently rebuilt and measured against pdflatex (TeX Live 2026) rather than just trusting the PR's own table; also diffed against a main-branch build. No blocking issues. Two non-blocking observations below (most notable first), then the checks that fully passed.

  1. Non-blocking, informational — the PR's "Not done" note undersells scope. The PR says the stale Nucleus::GenFraction doc comment at crates/compiler/src/math.rs:140 ("unlike the plain \frac's inner Nucleus::Fraction") is "left for a compiler PR" as if it's just a comment. It isn't only a comment: crates/compiler/src/math.rs:3256 (fn atom_class, Nucleus::Fraction { .. } => Inner) still feeds a live width computation — spacing_classes (math.rs:3313) → inter_atom_mu (math.rs:3339) → out.width += inter_atom_mu(...) (math.rs:3422) inside layout_list_with_scales, which backs crates/compiler/src/layout.rs's own greedy layout engine (used internally for \ref/\pageref page-number resolution via crates/compiler/src/xref.rs/parser.rs, and covered by layout.rs's own fraction_stacks_smaller_children_and_advances_as_one_unit test). This is identical in the vendored copy the render-pipeline actually builds against (crates/render-pipeline/vendor/compiler/src/math.rs:3256, byte-identical). So after this PR, crates/compiler's own layout still computes \frac's width as if it were Inner, while render-pipeline's typeset output now treats it as Ord — a real (if out-of-scope, given the vendor-pin boundary) discrepancy, not merely stale prose. Practical impact is almost certainly negligible (≤1.8bp per adjacent-Ord occurrence, very unlikely to move a page break), but whoever picks up "a compiler PR" for the doc comment should know it's a functional fix, not a wording fix.

  2. Verified non-issue, but the PR's \over claim doesn't match the actual code path (checked per item 2 of the review request). \over and \frac parse to the exact same AST node with no discriminating field: bare \over/\choose builds Nucleus::Fraction { numerator, denominator } at crates/compiler/src/math.rs:753 (inside the infix == "choose" || infix == "over" arm at line 744), and \frac builds the identical variant shape at crates/compiler/src/math.rs:1462 (both leave class_override: None). render-pipeline/src/typeset.rs:6578's N::Fraction arm is the only conversion site and unconditionally sets frac.class = ml::AtomClass::Ord for both. So the PR's claim "math-layout's Atom::frac stays Inner, which is correct for a primitive \over" is true only of the unmodified Atom::frac constructor (math-layout/src/mathlist.rs:370, untouched) — not of what render-pipeline actually emits, since the override fires for \over too.
    I checked this empirically rather than taking the code-reading at face value, since a bare \over always consumes its whole enclosing group (math.rs:744-763 returns early, replacing the entire list), so its class is only ever observable when a user explicitly re-wraps it in extra braces next to a neighbour, e.g. {a \over b}\text{as } x\to 0. Measured that exact source:

    • pdflatex: aas gap 6.962 bp
    • this PR's branch: 6.962 bp (exact match)
    • main: 8.780 bp (i.e. main had the same +1.818 bp Inner–Ord thin-space bug for bare \over that this PR documents for \frac, just never called out)
      So the blanket override is empirically correct for \over too (and is a bonus fix main never got credit for) — but that's because TeX itself promotes a braced \over subformula to Ord, not because the code distinguishes \frac from \over (it can't). \atop/\above aren't parsed as commands at all in this compiler (no match arms found), so they're not a concern.
  3. \binom/\genfrac/\dfrac/\tfrac code paths: confirmed untouched — no genfrac/binom hits in the diff, and they go through the separate Nucleus::GenFraction/N::GenFraction arms (typeset.rs:6366, 7122, etc.), not N::Fraction. Pass.

  4. \frac in script position: rendered $x^{\frac12}$ and $a_{\frac{1}{2}}$ on both the PR branch and a main-branch build — glyph positions are bit-for-bit identical between the two (superscript run at x=150.165/y={133.636,140.020}, subscript run at x=149.697/y={140.837,147.221} on both). Ord classification has no observable effect on script attachment, as expected. Pass.

  5. Ord–Bin and Ord–Rel spacing around \frac, measured independently (own minimal docs, not the PR's), pdflatex vs this PR's branch:

    • \[a+\frac12\]: pdflatex +→frac-nucleus gap 12.107 bp, PR branch 12.104 bp (Δ0.003 bp)
    • \[\frac12=b\]: pdflatex frac-nucleus→= gap 9.680 bp, PR branch 9.680 bp (Δ0.000 bp); =b 11.518 vs 11.515 (Δ0.003 bp)
      All well within the requested ≤0.1 bp tolerance, and identical to a main-branch build of the same docs (expected: TeX's spacing table gives Bin/Rel the same mu whether the other side is Ord or Inner, so this pairing was never at risk from the class change). Pass.
  6. amsmath oracle corpus (crates/compiler/tests/amsmath_corpus, python3 oracle.py check --render <flashtex-render> --fonts apps/mac/Fonts): 59 fixtures on disk, matching the PR's claim. This PR's branch: TOTAL 59/59 within 0.5 bp. Main branch, same harness: also TOTAL 59/59 within 0.5 bp — no delta; the corpus doesn't happen to exercise the specific adjacent-Ord pattern the PR fixes. Pass.

  7. cargo test --release --test frac_ord_spacing (crate flashtex-render-pipeline): reproduced, test result: ok. 1 passed; 0 failed; 0 ignored. Pass.

@GoKubar
GoKubar merged commit b01ba55 into main Sep 15, 2026
13 of 21 checks passed
@GoKubar
GoKubar deleted the agent/daniel-parent/display-frac-quad-gap branch September 16, 2026 04:17
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