Skip to content

importer: shared TeX-plan library + parsing fixes from real-blueprint dogfooding - #6

Merged
lolipopshock merged 3 commits into
mainfrom
fix/importer-tex-parsing
Jul 14, 2026
Merged

importer: shared TeX-plan library + parsing fixes from real-blueprint dogfooding#6
lolipopshock merged 3 commits into
mainfrom
fix/importer-tex-parsing

Conversation

@lolipopshock

Copy link
Copy Markdown
Contributor

Summary

Second PR in the migration-fix series (after #5). Every fix below was reproduced by importing two real blueprints — RemyDegenne/brownian-motion (a \part{}-structured, multi-file plan) and thefundamentaltheor3m/Sphere-Packing-Lean (plastex split-level=1) — into fresh template copies.

New scripts/lib/tex-plan.mjs — TeX-plan parsing toolkit (comment stripping, recursive \input resolution, \newcommand/\DeclareMathOperator expansion, environment/directive parsing with brace-aware heading titles, single-quoted YAML scalars), unit-tested, and shared with the upcoming native-chapter migration script.

Fixes in import-blueprint.mjs:

  • Plan mode failed with no items parsed on \input-sequence layouts (the one leanblueprint's own template comments recommend) — inputs now resolve recursively; --macros= expands project shorthands.
  • Frontmatter titles containing LaTeX math ($\mathcal{V}$) emitted invalid double-quoted YAML and broke the site build; fm() now single-quotes.
  • Scrape mode read only sub-toc-0, so \part-structured sites got part titles glued onto chapters 1–2 and Chapter N fallbacks for the rest; the chapter level is now the deepest integer-numbered TOC level (LaTeX chapter counters run globally through parts, matching the dep graph's N.M item numbers).
  • --base-url no longer silently defaults to the Sphere-Packing site, and a missing --label derives from the published site's <title>.
  • Importing over the demo leaves lakefile.toml roots naming deleted .lean chapters (CI fails later, silently) — now detected with a loud warning pointing at the adoption tutorial.
  • \mathlibok/\notready markers are stripped instead of leaking into statements verbatim.
  • hast-util-from-html declared (was an undeclared transitive dependency).
  • main() import-guarded; fm/parseChapterToc exported for tests; --chapter-level=section for split-level-1 plans.

Verification

  • npm run check and full test suite pass (112/112, including 10 new tests: \input chains, macro args/optional-defaults/operators, marker stripping, \part + flat TOC fixtures, YAML escaping).
  • Live scrape of the brownian-motion site: 15 chapters, all real titles, derived label, stale-roots warning fires.
  • Plan mode on its raw multi-file content.tex: 620 items / 15 chapters with no manual flattening.

🤖 Generated with Claude Code

… dogfooding

New scripts/lib/tex-plan.mjs (comment stripping, recursive \input resolution,
custom-macro expansion, directive/environment parsing with brace-aware heading
titles, single-quoted YAML scalars) with unit tests; import-blueprint.mjs plan
mode now consumes it. Fixes, each reproduced on RemyDegenne/brownian-motion or
Sphere-Packing-Lean:

- plan mode failed with 'no items parsed' on multi-file blueprints (content.tex
  as a pure \input sequence — the layout leanblueprint itself recommends);
  \input is now resolved recursively, and --macros=a.tex,b.tex expands the
  project's custom shorthands so they don't leak into statements raw
- frontmatter titles with LaTeX math broke the site build ('unknown escape
  sequence' — double-quoted YAML treats backslash as an escape); fm() now emits
  single-quoted YAML scalars
- scrape mode titled chapters after \part entries and lost every real chapter
  name ('Chapter 3'…'Chapter 15' on brownian-motion): parseChapterToc read only
  sub-toc-0; the chapter level is now the deepest integer-numbered TOC level
- scrape mode defaulted --base-url and the label to the Sphere-Packing site;
  --base-url is now required and a missing --label derives from the site title
- after an import replaces the demo blueprint, lakefile.toml roots still name
  the deleted .lean chapters and lake build / CI fail silently later — the
  importer now prints a loud warning with the docs pointer
- \mathlibok / \notready marker macros are stripped instead of leaking into
  rendered statements
- hast-util-from-html was imported but undeclared (worked via hoisting luck);
  now a real dependency
- main() is guarded so tests can import fm/parseChapterToc; --chapter-level=
  section supports plastex split-level=1 blueprints in plan mode

Verified: 112/112 tests; live scrape of brownian-motion now yields 15 correctly
titled chapters + derived label + stale-roots warning; plan mode on its raw
multi-file content.tex yields 620 items / 15 chapters with no manual flattening.

Co-Authored-By: Shannon's Claude <257597027+shannonshen49@users.noreply.github.com>
@sepo-agent-app

Copy link
Copy Markdown

Do you want Sepo to review this PR?

Add the agent/review label or comment @sepo-agent /review to launch an agent review.
For a preview deployment, add the sepo-preview label.

Comment thread scripts/lib/tex-plan.mjs Outdated
const args = []
if (def.optDefault !== null) {
if (src[j] === "[") {
const close = src.indexOf("]", j)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When src[j] === "[" but no matching ] exists, indexOf returns -1; this slices nearly the remaining source as the optional arg and resets j to 0, so a malformed call like \ball[r{x} expands to corrupted output (B_{r{x}(\)ball...). Please guard close === -1 and either use the default optional arg, leave the macro unexpanded, or fail fast, with a regression test.

Comment thread scripts/lib/tex-plan.mjs Outdated
const chapters = []
const parts = []
let current = null
const T = "(?:[^{}]|\\{[^{}]*\\})*"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

T only accepts one nested brace group inside a heading. A valid title like \chapter{A \frac{\sqrt{n}}{2}} does not match the chapter arm, so the following items are parsed under fallback Blueprint. Since this file already has readBalanced, can the heading scan use balanced groups instead of a bounded-depth regex?

@sepo-agent-app

This comment has been minimized.

@lolipopshock

Copy link
Copy Markdown
Contributor Author

Re the INFO finding on missing \input files: warn-and-continue is deliberate — a partial source checkout still yields a reviewable import, and the warning names each skipped file. A --strict mode that fails instead is a reasonable follow-up if partial imports bite in practice.

Comment thread scripts/lib/tex-plan.mjs
args.push(g.inner)
j = g.end
} else {
args.push(src[j])

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 branch treats every unbraced required argument as one character. In TeX, an unbraced argument can be a full control sequence, so a valid call like \norm\mu for \newcommand{\norm}[1]{...#1...} becomes corrupted output. Please consume a control word/control symbol as one argument token when src[j] === "\\", or leave the macro unexpanded if the argument token is incomplete, and add a regression test.

@sepo-agent-app

Copy link
Copy Markdown

AI Review Synthesis

Dual-agent review by Claude and Codex.
Requested by @lolipopshock.

Summary of PR/Issue

Review

The PR is close, but several parser edge cases can silently corrupt imported TeX or blueprint structure. The most important fixes are localized and suitable for an automated follow-up.

Issue Severity Description
Unbraced macro control-sequence arguments are corrupted WARNING expandMacros treats every unbraced required argument as one character, so valid TeX such as \norm\mu expands incorrectly.
Optional macro argument without closing bracket corrupts expansion WARNING When src[j] === "[" and no ] exists, indexOf returns -1, causing bad slicing and corrupted output.
Deeply nested heading titles parse under fallback Blueprint WARNING The heading regex only accepts one nested brace level, so titles like \chapter{A \frac{\sqrt{n}}{2}} are skipped and later items attach to fallback Blueprint.
New importer flags are not documented in user docs WARNING --macros and --chapter-level are only visible in the script header even though --chapter-level=section fixes a reproduced split-level=1 workflow.
Same-line repeated \input is only partially resolved INFO resolveInputs only replaces a line-leading \input, so a second \input on the same line remains verbatim; impact is low because leanblueprint convention uses one input per line.

Progress

  • Inspected both current review artifacts, the PR conversation, current head ffa0b14407edfa7d2362399164344194a01fade4, changed files, inline comments, and review threads.
  • Existing same-agent inline threads already cover scripts/lib/tex-plan.mjs:125 and scripts/lib/tex-plan.mjs:207; I did not duplicate them.
  • Posted one new inline comment for the non-duplicated scripts/lib/tex-plan.mjs:138 macro-tokenization issue.
  • No inline replies, minimizations, or thread resolutions were performed.
  • The author clarified that warn-and-continue for missing \input files is deliberate, so that INFO note is not carried forward as required work.

Issue Details

Unbraced macro control-sequence arguments are corrupted

Cause: In expandMacros, the fallback required-argument path consumes only src[j].

Candidate solutions: When src[j] === "\\", consume a full TeX control word/control symbol as the argument token, or leave the macro unexpanded if the token is incomplete. Add a regression test for \newcommand{\norm}[1]{...#1...} with \norm\mu.

Optional macro argument without closing bracket corrupts expansion

Cause: src.indexOf("]", j) can return -1, after which slicing and j = close + 1 corrupt the expansion.

Candidate solutions: Guard close === -1 and either use the default optional argument, leave the macro unexpanded, or fail fast. Add a malformed optional-argument regression test.

Deeply nested heading titles parse under fallback Blueprint

Cause: T encodes bounded brace depth in a regex.

Candidate solutions: Parse heading arguments with the existing readBalanced scanner instead of the bounded-depth regex. Add a regression test with a deeply nested heading title.

New importer flags are not documented in user docs

Cause: The new flags are present in the script usage but absent from the migration/reference docs.

Candidate solutions: Document --macros and --chapter-level=chapter|section in docs/tutorial/quick-start/migrate-existing-blueprint.md and/or docs/documentation/reference.md.

Comments: Reviewers differed on severity here: one rated this WARNING, one INFO because the importer is legacy. Synthesis keeps it WARNING because --chapter-level=section is the practical workaround for a reproduced split-level=1 import mode.

Same-line repeated \input is only partially resolved

Cause: resolveInputs matches only line-leading \input directives, so \input{a}\input{b} resolves the first and leaves the second in the source.

Candidate solutions: Optional hardening: scan all directive tokens on the line or explicitly document one-\input-per-line input.

Comments: This is INFO-level and not recommended as required automation work.

Recommended Next Step

FIX_PR: The unresolved WARNING findings are concrete parser and documentation changes that are safe for an automated fix-pr pass.

Final Verdict

MINOR_ISSUES

Action Items

  • Fix unbraced macro argument tokenization in expandMacros and add a regression test.
  • Guard missing optional-argument closing brackets in expandMacros and add a regression test.
  • Replace bounded-depth heading parsing with balanced-group parsing and add a nested-title regression test.
  • Document --macros and --chapter-level in the migration/reference docs.

codex | gpt-5.5/xhigh | Macmini-runner-5

…lan-mode flags

Review findings on the shared parser:
- expandMacros consumed one character for unbraced required args, corrupting
  \norm\mu-style TeX; an unbraced control sequence is now taken whole.
- An unterminated optional [arg falls back to the macro default instead of
  slicing to -1 and corrupting the output.
- Heading titles now nest braces two levels (\chapter{A \frac{\sqrt{n}}{2}});
  one level made such chapters fall through, attaching their items to the
  previous chapter.
- migrate-existing-blueprint.md documents --macros and --chapter-level for
  plan mode (previously only in the script header).

Same-line repeated \input stays line-leading-only by design (leanblueprint
convention is one \input per line).

Co-Authored-By: Shannon's Claude <257597027+shannonshen49@users.noreply.github.com>
@lolipopshock

Copy link
Copy Markdown
Contributor Author

Round-2 findings addressed in 56028a0: unbraced control-sequence args (\norm\mu) now consume the whole control sequence; an unterminated optional [ falls back to the macro default instead of slicing to -1; heading titles nest braces two levels (\chapter{A \frac{\sqrt{n}}{2}} no longer falls through to the previous chapter); --macros/--chapter-level documented in the migrate tutorial. Same-line repeated \input stays line-leading-only by design (leanblueprint convention is one per line) — noted in the code comment.

… plan-mode flag paragraphs)

Co-Authored-By: Shannon's Claude <257597027+shannonshen49@users.noreply.github.com>
@lolipopshock
lolipopshock merged commit 91ca982 into main Jul 14, 2026
4 checks passed
@sepo-agent-app

Copy link
Copy Markdown

Rubrics Update

No changes were committed to agent/rubrics from PR #6.

Added one active rubric:

rubrics/coding_workflow/blueprint-importer-partial-inputs.yaml

It captures the trusted maintainer preference from PR #6: blueprint importer work should preserve warn-and-continue behavior for missing TeX \input files, and should not assume same-line repeated \input support unless explicitly requested.

No commit was made.


codex | gpt-5.5/xhigh | Macmini-runner-2

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.

1 participant