util: bound what equation() renders, and make rendering it in full cheap (fixes #582)

[Yaraslaut]

Fixes #582. Fixes #602.

#582 asked for the depth cap #574 deferred. #574 deferred it because it changes
equation()'s documented output contract rather than fixing a defect — so the
engineering here is the small half. The decisions are below, and they are in
docs/spec/util/quantity_type.md ("The rendering is bounded; the derivation is
not") rather than only in this description.

1. What the limit is, and what N is

equation(std::size_t maxSteps = kDefaultEquationSteps), with
kDefaultEquationSteps == 100.

A step is one operation node written out. Atoms (leaves, conversions) and
named nodes render as a single token and cost nothing; only a node whose
operation is spelled out does. The budget is spent once per call, across the
formula and the legend together, in the pre-pass that already assigns cN
placeholders — so the symbolic rendering, the substituted rendering and every
legend line consult one shared decision and cannot disagree about what was
written out. A per-rendering budget could not guarantee that, because the legend
renders subtrees the formula stops short of.

Why 100, and why that is not an arbitrary constant. It is a readability
bound, not a cost bound, and the two would give very different numbers:

• The defect util/render: two reproduced value-type defects that ship by default — Quantity provenance chains and unvalidated locale group-separator stripping #574 named is that the output is not an explanation. A rendered
  formula stops being one long before it stops being affordable — 100 written-out
  steps is already more than anyone reads, and two orders of magnitude above any
  derivation in this repository's examples. Setting the default where the cost
  becomes intolerable instead (thousands of steps, on the measurements below)
  would keep producing output nobody can use, which closes the ticket without
  fixing the thing it is about.
• The two errors are not symmetric, and this is the load-bearing half. Because
  the limit is a parameter, a default set too low costs a caller one argument. A
  default set too high costs everyone the half-megabyte line, and they cannot opt
  out of what they never knew was unbounded. So the default sits at the human end
  of the range.

I did not come back for product input on N, and the reasoning above is why:
the question "what is the right N" only needs a product answer if N is fixed.
Making it a parameter turns it into "what default is safest", which is
answerable from the asymmetry, and leaves the actual policy where invariant 3
puts it — see §3.

2. What the collapsed output looks like, exactly

Past the limit a sub-derivation is elided: eK in the formula (e1, e2, …,
numbered in first-appearance order exactly as cN is), its value in the
substitution, and one legend line of its own. Real output, 70,000-step chain,
default limit:

e1 + c1 + c1 + c1 + … + c1
    = 69900 + 1 + 1 + 1 + … + 1
    = 70000
where c1 = 1
      e1 = 69900 (elided at the 100-step limit)

• The legend line is self-describing and names the limit, so a caller meeting
  an e1 for the first time can tell it from an ordinary value without reading
  the spec. That is the whole point of not simply truncating to the value.
• The result line is untouched. Eliding changes the account of how a value
  was reached, never the value.
• Elision beats a placeholder: a node that is both reused and past the limit
  gets an eK, not a cN, because a cN's legend line would expand the very
  subtree the limit just declined to render. (This is also a correctness
  constraint, not only taste — atomRendering has to test elision before
  isPlaceholder, or it looks up a cN label that was deliberately never
  assigned.)
• The walk is not what the limit bounds. Reference counting still visits
  every node: reuse is a property of the derivation, not of how much gets
  printed, and a value shown once must not earn a placeholder because the limit
  hid its other uses.
• Where the cut falls is a choice, and it is in the spec: the walk is a
  left-before-right pre-order, so what survives is the steps nearest the result
  and what collapses is the deep end. On the running-total shape that is the
  right way round — the last 100 additions stay legible, the accumulated history
  folds into one number.
• kEquationStepsUnlimited restores the pre-util: equation() renders an unbounded derivation in full, quadratically — #574's deferred depth cap #582 rendering. 0 returns the
  formatted value alone — the same one-element answer a tracing-off build gives,
  which is the only coherent reading of "show me no steps".

3. Configurable, and the reasoning rather than the assertion

Configurable. The triage skill's invariant 3 — the value type reports the fact,
the layer decides the policy — is the argument for it, and I think it holds
here rather than being invoked: how much of a derivation is worth reading is a
property of who is reading. An audit log wants all of it; a tooltip wants a
line. Quantity cannot know which it is talking to, and a fixed constant would
make that guess on every caller's behalf, permanently.

The argument against, weighed: a default argument is API surface, and an API
that asks a question most callers do not want to answer is a cost. That is real,
and it is why this is a defaulted parameter and not a required one — the
common caller writes equation() and gets a sensible answer, exactly as before.
It is also why there is no options struct, no builder and no global setting: one
std::size_t with a default is the smallest thing that moves the policy to the
layer that owns it.

4. Whether the cap is the whole fix — no, and the issue was right about that

#582 says there are two problems and they may want different answers. They do.

• "It is not an explanation" — the step limit fixes this.
• "It is quadratic" — the step limit only bounds this, and it stops being
  bounded the moment a caller asks for more. So combine now takes its left
  operand by value and appends to it instead of concatenating both sides
  into a fresh string. The left operand of a left-leaning chain — the shape
  total = total + row records — is the whole expression rendered so far, and
  copying it once per level was the quadratic term. Appending makes that shape
  linear.

I did not reserve, which was the issue's other suggestion: reserving cannot help,
because the cost is the copy itself, not the reallocation. And the append does
not fix every shape — a right-leaning chain (a + (b + (c + …))) and a chain
of unary negations still copy the big operand per level and are still quadratic,
because prepending is inherently O(n). The step limit is what bounds those, and
the spec says so rather than leaving it implied.

Measurements

Reproduced the issue's own numbers first, then measured the same cases after.
clang 22.1.8, -O1, ASan+UBSan, total = total + one n times then
equation(); median of three runs. "chars" is the length of the first output
line.

n	master be64026a (the base these were taken on; rebased since onto fbb90181, which touches nothing in morph::units)	this branch, default (100)	this branch, kEquationStepsUnlimited
40,000	7.544 s / 200,001 chars	0.020 s / 502 chars	0.074 s / 200,001 chars
70,000	27.720 s / 350,001 chars	0.024 s / 502 chars	0.112 s / 350,001 chars
100,000	58.837 s / 500,001 chars	0.057 s / 502 chars	0.157 s / 500,001 chars

Real output at n = 100,000, after:

n=100000 build=0.031s equation=0.057s lines=5 first=502 chars
  [0] e1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 + c1 +  ...
  [1]     = 99900 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 +  ...
  [2]     = 100000
  [3] where c1 = 1
  [4]       e1 = 99900 (elided at the 100-step limit)
destroyed cleanly

The unlimited column is the append's doing alone, not the limit's: 27.7 s → 0.11 s
at 70,000 with byte-identical output.

Non-vacuity: proved by mutation, not asserted

The step limit. Mutated EquationRenderer's constructor so the budget never
exhausts (stepBudget(SIZE_MAX)), leaving every other line of the feature in
place, and rebuilt. Four assertions fail, and every one of them is about the
size of the output:

equation() renders a deep derivation within the default step limit
  REQUIRE( lines.size() == 5 )  with expansion: 4 == 5
equation() numbers several elisions in first-appearance order
  REQUIRE( lines.size() == 5 )  with expansion: 3 == 5
equation()'s step limit is the caller's to set / one step past it elides exactly one sub-derivation
  REQUIRE( lines.size() == 5 )  with expansion: 4 == 5
equation()'s step limit is the caller's to set / a caller that wants less says so
  REQUIRE( lines.size() == 5 )  with expansion: 4 == 5

The sections that keep passing under the mutant are the ones that assert the
absence of elision (at exactly the limit, below it, unlimited, zero) — they
should, and they are the guard against the limit firing early.

The assertions spell the whole expected formula out rather than matching a
prefix. A starts_with("e1 + c1") check would pass just as happily against a
350,001-character line, which is precisely the failure this is about.

The append. No test asserts linearity directly, and I am not going to
pretend otherwise — a timing assertion is flaky. What guards it is ctest's 120 s
timeout on the uncapped 70,000-step test, and that guard is real rather than
nominal: reverting combine to the copying form takes that render from 0.11 s to
27.7 s under ASan+UBSan and from 1.3 s to over 120 s at -O0 under TSan, which
is how #589 found it in the first place. Locally, non-sanitized, the same mutation
moves the test binary from 0.376 s to 0.956 s. It is a coarse guard (120 s
against 1.3 s), and it is the only one.

#590's 70,000-node test

Two things happened to it, and the first matters more than the timing.

It had to be changed or it would have gone vacuous. Under the default limit
the renderer stops 100 steps in and never walks deep enough to overflow
anything — so the test would have passed against the recursive code it exists to
catch. It now asks for kEquationStepsUnlimited explicitly, with the reason in a
comment above the call. This is the AGENTS.md "would the check still pass if the
feature did nothing" question applied to the old test rather than the new ones,
and the answer was yes.

The timeout can be tightened, so it is. #590 gave the test a [slow] tag and
a 600 s ctest TIMEOUT because #582's quadratic cost pushed it past 120 s under
TSan. Measured after the append, same shape, harness at -O0 under TSan:
1.26 s (median of three) against a >120 s failure before. Confirmed against the
real CI build rather than only the harness — ctest on the clang-tsan preset
reports that test at 1.14 s, and on clang-asan at 1.11 s:

16/17 Test #920: equation() walks a 70000-node provenance chain without overflowing the stack ...   Passed    1.14 sec

So the [slow] tag and
the separate catch_discover_tests registration it keyed on are gone, and
tests/CMakeLists.txt is back to one call with one 120 s cap — a ~95× margin.
Removing the tag and the TEST_SPEC "~[slow]" filter together is deliberate:
leaving the filter behind with no tagged test would silently drop any future
[slow] test from registration.

#602, filed and fixed here

Found while building this: assignLabels was the one walk in equation()
without a visited set, so a node reachable by k displayed paths was walked k
times. On a DAG that is exponential — 31 nodes built by repeated q = q + q
have 2³⁰ root-to-leaf paths and took 10.3 s to render 33 short lines,
measured on master. Filed separately as #602 with its own evidence, per
AGENTS.md, rather than buried in this description.

It is fixed in this PR rather than deferred because the step limit depends on
it: without the visited set the budget is spent on repeat visits of the same
nodes, and the limit would fire on derivations far smaller than 100 distinct
steps — a 31-node DAG would elide. That makes it part of this change rather than
a separate finding folded in. The fix is behaviour-preserving (a second visit can
only re-assign labels the node already holds), and the evidence for that is
byte-identical output before and after; 61 nodes / 2⁶⁰ paths now render instantly.
tests/test_quantity.cpp gains a 41-node regression case whose failure signal is
the 120 s timeout, as the #574 cases' is a segfault.

Review notes

No Copilot seats on this org, so the review reasoning is here. What I looked at,
in the code rather than in the diff:

• Do the two renderings agree? They must, or labelIndex.at() throws. Both
  atomRendering modes stop at exactly the same node set — named, placeholder,
  leaf, conversion, elided — and the one asymmetry (expandThis on a named node)
  cannot arise, because expandThis is set only for the root and for legend
  lines, equation() returns early on a named root, and a named node never earns
  a label. Elision is decided once in the pre-pass and read by both, so they
  cannot drift.
• Can refCount.at() throw on a new path? It is total for children, and the
  root is the only node not in it. isPlaceholder is still reached only behind
  !expandThis. The elision check is placed before it precisely so a
  reused-and-elided node does not take the placeholder path.
• Can a node be rendered that the pre-pass never visited? render descends
  into a strict subset of what assignLabels descends into (it also stops at
  placeholders), so no.
• Is the pending stack still bounded? With a limit, the frontier is at most
  N+1 entries. Without one it is the old behaviour, which is iterative and was
  the point of Fix two shipped value-type defects: provenance chains that overflow the stack, and locale group separators that are stripped instead of validated #581.
• Is combine's move safe at the call site? top.left is moved from and
  the frame is popped on the next statement; finished is passed as the
  const-ref right operand and assigned only from the returned prvalue.
• Does the tracing-off build still compile? Yes — -DMORPH_QUANTITY_PROVENANCE=0
  syntax-checks clean; the parameter is [[maybe_unused]] there, since that
  build has no derivation to limit.

Verification

• ctest on clang-tsan and on clang-asan, with CI's own
  -E "OomInjector|morph#108" exclusion: 1524/1524 pass on each. (Without
  the exclusion both legs fail the five OomInjector cases and morph#108 —
  pre-existing and why CI excludes them: the injector's operator new override
  is compiled out under a sanitizer.)
• ctest on clang-debug: 1530/1530 pass (1529 before this branch; three
  new test cases, one of which registers as several sections).
• clang-tidy-diff over master...HEAD, changed lines, clang 22.1.8: clean.
  Three findings on the first pass (rvalue-reference-param-not-moved,
  identifier-length, pro-bounds-avoid-unchecked-container-access) fixed
  rather than suppressed — combine takes its operand by value, it became
  elided, and the legend loop no longer indexes.
• clang-format --dry-run --Werror on the changed sources: clean.
• Doxygen with WARN_AS_ERROR = FAIL_ON_WARNINGS: builds.
• scripts/check_spec_sync.sh against the real diff: 10 sub-domain(s) classified; every touched header sub-domain has a matching spec change.
• scripts/check_spec_citations.sh, check_catch_test_names.sh,
  check_ctest_name_collisions.sh: clean.
• Compiler cache: fastcache-cc (verified in build.ninja), not ccache.
• scripts/branch_partial_allowlist.json is untouched — no entry cites a
  line in either changed header, and line is a hint rather than the key, so
  nothing needed refreshing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GS5K2vqZtC4xbRiGJHT7jH

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!