Revert "MTMacroAtom and two-phase finalized (#265)"

[kostub]

Reverts #265.

#265 added MTMacroAtom, the kMTMathAtomMacro type, a two-phase -finalized (expandMacros, then the existing reclassifying pass), and -transferScriptsToExpansion:, so that \pmod{n} would serialize back to \pmod{n} rather than to its expansion. Nothing else uses any of it, and \pmod/\mod/\pod themselves had not landed yet — so on master today this is ~460 lines of model-layer machinery with no caller.

Why the requirement it served isn't one

Preserving the command name through serialization is not an acceptance criterion for the feature. PRD §10 lists eight; none concern serialization. It appears only as an open question for the LLD (§9.5), phrased conditionally. Meanwhile PRD §11's own reference table already describes \pmod's iosMath atom head as Space + Open + Ord … — an atom sequence, i.e. the expansion. And iosMath already declines exactly this fidelity elsewhere: \implies serializes as \Longrightarrow.

What replaces it

\pmod, \mod and \pod land in #266 instead as a parse-time expansion, the way TeX defines them — a three-entry table of prefix/suffix LaTeX strings and one method that reads the argument and splices it between them, entirely inside MTMathListBuilder.

That turns out to be the more faithful of the two designs, not a compromise:

	with the macro atom	parse-time expansion
\pmod{n}^2	^2 transferred onto the last scriptable atom of the expansion	^2 on the ), as in LaTeX
nesting	expansion pass + depth budget	falls out for free

The cost is that mathListToString emits the expansion rather than the command. It is equivalent LaTeX that re-parses to a structurally identical atom list, which #266 pins with a test.

Scope

MTMathList.h and MTMathList.m return to their state at e278be3. The \bmod work from #264 is untouched. 446 tests, 0 failures; release build clean.

#266 will be rebased onto this.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ts3f5UtUywaqimvE4U1rkw

Summary by CodeRabbit

• Breaking Changes

  • Removed support for representing and expanding unexpanded macro invocations as specialized math atoms.
  • Removed the associated public macro-atom API and internal macro-parameter handling.

• Improvements

  • Simplified math-list finalization and preprocessing behavior.
  • Improved Unicode text handling support for typesetting.
  • Updated project configuration and test coverage to reflect the revised math-processing behavior.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR removes macro atom declarations, macro expansion logic, and associated tests. It updates header search paths and Unicode header wiring, removes obsolete internal headers, and replaces the removed test target entry with MTTypesetterTest.m.

Changes

Macro removal and project cleanup

Layer / File(s)	Summary
Remove macro atom model and expansion iosMath/lib/MTMathList.h, iosMath/lib/MTMathList.m	Removes kMTMathAtomMacro, MTMacroAtom, MTMacroParameterAtom, macro expansion helpers, and macro expansion from finalization.
Update header and atom-type integration iosMath/lib/MTUnicode.h, iosMath/module.modulemap, iosMath/render/internal/MTTypesetter.m, Package.swift, iosMath.xcodeproj/project.pbxproj	Adds the Unicode category declaration, updates its include and modulemap references, and removes lib/internal header search paths and obsolete internal group entries.
Replace removed test target coverage iosMathTests/MTModularArithmeticTest.m, iosMath.xcodeproj/project.pbxproj	Removes macro-related modular-arithmetic tests and replaces the Xcode test-source entry with MTTypesetterTest.m.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• kostub/iosMath#264: Both changes modify modular-arithmetic tests in MTModularArithmeticTest.m.
• kostub/iosMath#265: This PR removes the macro APIs and logic introduced by that change.
• kostub/iosMath#266: Both changes modify macro atom handling, expansion, and related modular-arithmetic tests.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states this PR reverts the MTMacroAtom/two-phase finalization change from #265.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch revert/macro-atom

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

iosMath.xcodeproj/project.pbxproj (1)

530-542: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Orphaned test file: MTModularArithmeticTest.m survives the revert but is no longer built. The revert stripped every project reference to the file while leaving the trimmed file (and its still-relevant \bmod tests) in the tree, so that coverage silently stops running.

• iosMath.xcodeproj/project.pbxproj#L530-L542: either restore the MTModularArithmeticTest.m build-file/file-reference/group/sources entries, or drop the file from the tree in this PR.
• iosMathTests/MTModularArithmeticTest.m#L141-L142: if the file is intentionally retired, relocate the surviving \bmod tests (including testDemotedBmodSerializes) into a test file that remains in the target before deleting it.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@iosMath.xcodeproj/project.pbxproj` around lines 530 - 542, Restore
MTModularArithmeticTest.m’s file-reference, build-file, group, and
PBXSourcesBuildPhase entries in iosMath.xcodeproj/project.pbxproj so the test
target builds it, or intentionally remove
iosMathTests/MTModularArithmeticTest.m; if removing it, first move its surviving
\bmod tests, including testDemotedBmodSerializes, into a test file that remains
in the target. The project file site requires the build-reference restoration or
deletion decision, and the test-file site requires relocation before deletion.

🧹 Nitpick comments (1)

iosMath/render/internal/MTTypesetter.m (1)

15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the search-path import here (iosMath/render/internal/MTTypesetter.m:15). MTUnicode.h is already on the target’s header search paths, so #import "MTUnicode.h" is cleaner than ../../ traversal.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@iosMath/render/internal/MTTypesetter.m` at line 15, Replace the relative
traversal import in MTTypesetter.m with the search-path import for MTUnicode.h,
using the header name directly while preserving the existing dependency.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@iosMath.xcodeproj/project.pbxproj`:
- Around line 530-542: Restore MTModularArithmeticTest.m’s file-reference,
build-file, group, and PBXSourcesBuildPhase entries in
iosMath.xcodeproj/project.pbxproj so the test target builds it, or intentionally
remove iosMathTests/MTModularArithmeticTest.m; if removing it, first move its
surviving \bmod tests, including testDemotedBmodSerializes, into a test file
that remains in the target. The project file site requires the build-reference
restoration or deletion decision, and the test-file site requires relocation
before deletion.

---

Nitpick comments:
In `@iosMath/render/internal/MTTypesetter.m`:
- Line 15: Replace the relative traversal import in MTTypesetter.m with the
search-path import for MTUnicode.h, using the header name directly while
preserving the existing dependency.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cda51451-ee31-4573-bc96-be3399db82ab

📥 Commits

Reviewing files that changed from the base of the PR and between 8d43971 and b2680c6.

📒 Files selected for processing (9)

• Package.swift
• iosMath.xcodeproj/project.pbxproj
• iosMath/lib/MTMathList.h
• iosMath/lib/MTMathList.m
• iosMath/lib/MTUnicode.h
• iosMath/lib/internal/MTMacroParameterAtom.h
• iosMath/module.modulemap
• iosMath/render/internal/MTTypesetter.m
• iosMathTests/MTModularArithmeticTest.m

💤 Files with no reviewable changes (3)

• iosMath/lib/internal/MTMacroParameterAtom.h
• Package.swift
• iosMath/lib/MTMathList.h