fix(expr): bound regex cache and charge to memory budget

[seant-aws]

What was the problem/requirement? (What/Why)

The per-evaluation regex cache (HashMap<String, regex::Regex>) was unbounded
and invisible to the memory budget. A list comprehension with a per-iteration
pattern (e.g. '(?:a{100}){100}z' + string(i)) compiled and cached a new
~499 KB regex each iteration. At N=4000, the process consumed ~2 GB of RSS
while the evaluator reported ~262 KB peak_memory — an 8,000× underreport.
Neither memory_limit nor operation_limit prevented the allocation.

What was the solution? (How)

Four interlocking defenses:

1. Charge cache entries to the memory budget — each compiled regex charges
   REGEX_SIZE_LIMIT + pattern.len() to current_memory via check_memory.
2. Cap the cache at 32 entries — past the cap, patterns compile but aren't
   cached. Legitimate templates use 1–3 distinct patterns.
3. Lower per-regex size_limit from 1 MiB to 256 KiB — the adversarial nested
   repetition pattern is now rejected outright.
4. Charge operations proportional to pattern length on cache miss —
   count_ops(pattern.len()) makes compilation visible to the op budget.

Regex cache charges are preserved across the comprehension iteration baseline
reset (the current_memory = memory_baseline line now adds back the delta).

What is the impact of this change?

Templates with >~380 distinct regex patterns in a single expression will now
hit the memory limit. Templates with patterns whose compiled NFA exceeds 256 KiB
(e.g. (?:a{100}){100}) are now rejected. All conformance suite patterns and
realistic template patterns compile well under 256 KiB.

How was this change tested?

• 5 new regression tests in test_memory.rs asserting each mechanism
• 5,731 existing tests pass (0 regressions)
• Clippy clean
• Conformance suite: 1,156 passed, 2 failed (both pre-existing)
• Manual CLI verification: adversarial template N=200 (105 MB → rejected at 2 MB)

Was this change documented?

Spec update to specs/expr/evaluator.md (Regex Cache section) pending.

Is this a breaking change?

Potentially — templates relying on patterns with compiled NFA >256 KiB or >32
distinct patterns per expression will see new errors. These patterns are
adversarial by construction; no legitimate template should be affected.

[github-actions]

Memory charged for patterns past the cache cap is never released, so it accumulates without bound.

entry_cost is added to current_memory (and to regex_cache_bytes) unconditionally at lines 1711–1716, but the compiled Regex is only retained when regex_cache.len() < MAX_REGEX_CACHE_ENTRIES. Past the cap the returned Regex is dropped as soon as the calling function returns, yet the 256 KiB charge stays on the budget permanently — and it is re-charged on every subsequent miss for the same pattern.

Concretely, [re_findall("x", "p" + string(i % 100)) for i in range(500)] has only 100 distinct patterns and holds at most 32 compiled regexes (~8 MiB by this accounting), but charges 468 uncached compiles × 262 144 = ~123 MB and so fails against the 100 MB DEFAULT_MEMORY_LIMIT. The comprehension path makes this stick: regex_delta at line 1590 deliberately carries the growth forward across iterations, and regex_cache_bytes includes the uncached charges.

So the cap does not actually bound the accounted cost the way the comment claims ("bounds total cache memory to at most MAX_REGEX_CACHE_ENTRIES × REGEX_SIZE_LIMIT") — it bounds real memory but not charged memory, and charged memory is what rejects the expression.

Suggest only retaining the charge when the entry is actually inserted, e.g.:

if self.regex_cache.len() < MAX_REGEX_CACHE_ENTRIES {
    self.check_memory(entry_cost)?;
    self.current_memory = self.current_memory.saturating_add(entry_cost);
    self.peak_memory = self.peak_memory.max(self.current_memory);
    self.regex_cache_bytes = self.regex_cache_bytes.saturating_add(entry_cost);
    self.regex_cache.insert(pattern.to_string(), re.clone());
} else {
    // transient: verify headroom, do not retain the charge
    self.check_memory(entry_cost)?;
}

Related: using REGEX_SIZE_LIMIT as the flat per-entry charge means a one-character pattern is billed 256 KiB. That is the conservative direction for the limit check, but it is also what EvalResult::peak_memory reports, so any expression touching a regex now reports a peak that is ~250 000× the real cost for small patterns. Worth confirming that overstatement is acceptable for the metric, or charging a smaller estimate and keeping REGEX_SIZE_LIMIT purely as the ceiling.

[github-actions]

Spec drift: this behavior change is not reflected in specs/expr/. AGENTS.md requires spec and code to line up in the same commit, and this PR changes two documented limits plus adds a new one:

• specs/expr/function-library.md:80 still shows .size_limit(1 << 20) in the EvalContext default-impl snippet, and line 88 states "a 1 MiB compiled-program size limit". Both are now 256 KiB.
• specs/expr/architecture.md:24 (report note) and the "all documented constants/limits match exactly (… 1 MiB regex …)" claim in reports/expr-quality-evaluation-report.md:24 are now stale.
• specs/expr/evaluator.md:437-449 ("Regex Cache") describes an unbounded cache with no memory accounting. It needs the new MAX_REGEX_CACHE_ENTRIES = 32 cap, the per-entry memory charge, and the count_ops(len(pattern)) compile charge — the last one is user-visible, since it changes operation_count for every expression that compiles a regex (see the exact_regex_search 6→7 change in this diff).

Also: REGEX_SIZE_LIMIT is pub(crate) in a pub(crate) mod evaluator, so it is not public API and public-api.md needs no entry — but MAX_REGEX_CACHE_ENTRIES being a hard-coded 32 with no with_* builder knob is worth calling out in evaluator.md next to the other configurable limits, so users hitting the recompile cliff have something to read.

Anchoring here because this is the line that silently inherits the lowered limit.

[github-actions]

Lowering size_limit 1 MiB to 256 KiB rejects previously-valid patterns, and widens divergence from the Python reference implementation.

Python re has no compiled-program size limit at all, so this cap is already a Rust-only restriction; quartering it makes the window of "valid in openjd-model-for-python, rejected in openjd-rs" four times wider. The new regex_size_limit_rejects_large_nfa test demonstrates exactly such a pattern — legal Python re, and per this constant doc comment it was accepted by this crate before this change.

The stated justification ("Every pattern in the conformance suite ... compiles well under this limit") establishes that nothing currently tested regresses, but not that no deployed template regresses. Bounded counted repetition is not exotic — IPv4-style patterns with counted groups, and character-class-heavy Unicode patterns, grow the compiled program quickly. And a template author has no way to raise the cap, unlike memory_limit / operation_limit which both have with_* builders.

Two things would make this safer:

1. Since the point of this PR is that compiled regexes are now charged to the memory budget, the size limit is arguably no longer the primary defense — the budget is. Consider leaving size_limit at 1 MiB and letting the now-accounted memory limit do the bounding, so the DoS fix does not also carry a compatibility regression.
2. If the cap stays at 256 KiB, the error should read in template-author terms. It currently surfaces as Invalid regex: Compiled regex exceeds size limit of 262144 bytes., which sounds like the pattern is malformed rather than too complex for this implementation, and gives no hint that Python would have accepted it.

Either way this warrants a spec update (see the function-library.md comment) and, since it can reject input that used to evaluate, a callout in the commit body.

[github-actions]

pattern.len() is a poor proxy for compile cost, which leaves the operation budget as an ineffective backstop.

Regex compile cost is superlinear in pattern length for counted repetition: (?:a{100}){100} is 16 characters and compiles to a program the PR itself measures at ~499 KiB, yet it is charged 16 operations. With DEFAULT_OPERATION_LIMIT at 10 million, the op budget permits on the order of 600k such compiles — it is not what stops a compile bomb here. What actually stops it today is the memory charge: 100 MB / 262 KiB caps distinct compiles at ~381.

That matters because of the interaction with the cache-cap charge issue I raised on the insert (line 1722). Those two mechanisms are load-bearing for each other in a way that is easy to get wrong:

• Keep the unconditional charge (current code) and the memory budget bounds compile count, but legitimate expressions with more than ~380 distinct patterns are rejected even though real memory stays at 32 entries.
• Release the charge for uncached compiles and real memory is correctly bounded, but nothing bounds the number of compiles any more — len(pattern) ops is too cheap to stop it.

A charge derived from the compiled program rather than the source would resolve both. regex-automata exposes NFA::memory_usage(), and regex::RegexBuilder does not surface it, but regex_syntax is already a dependency and validate_regex_pattern already builds the HIR — so a cost estimate is available cheaply, e.g. hir.properties() combined with the product of Repetition bounds. Charging estimated_program_bytes as both the memory figure and (scaled) the operation count would make the flat 256 KiB constant unnecessary and make small patterns cheap again.

If that is more than this PR wants to take on, at minimum a comment noting that the memory charge — not the op charge — is what bounds compile count would keep a future change from removing the wrong one.

[github-actions]

This test does not test the thing its name claims. It only asserts that evaluation succeeds and that the result is a ListList — both of which hold identically if MAX_REGEX_CACHE_ENTRIES were removed entirely, or set to 1, or set to 1000. Nothing here observes the cap. It is a smoke test named as a cap test, so it will not catch a regression in the cap logic.

To actually pin the mechanism, assert something that differs across the boundary. The op charge is the observable signal: count_ops(pattern.len()) fires once per compile, and past the cap every reuse is a fresh compile. So 32 distinct patterns reused N times charges 32 compiles, while 40 distinct patterns reused N times charges 32 + (8 x N). Something like:

// 40 distinct patterns, each reused twice. First 32 cache (1 compile
// each); the last 8 miss on every use (2 compiles each).
let ops = op_count("[re_findall(\"x\", string(i % 40)) for i in range(80)]");

and compare against the same expression with % 20, where every pattern caches.

Two smaller notes on the surrounding block:

• The comment "Pattern "0" matches 'x'? No. So all 50 results should be empty lists" reasons about behavior the test never checks. If empty results matter, assert them; if they do not, the comment is noise that will mislead a later reader into thinking they are covered.
• regex_cache_hit_not_recharged (line ~836) has the mirror-image problem: assert!(r.peak_memory < 1_000_000) is satisfied by any peak under the limit, and since the whole expression evaluated successfully under a 1 MB limit that is already implied. An equality-style assertion against the single-compile cost (or a comparison against the distinct-pattern variant) would pin the cache hit.

[github-actions]

This comment says the real cost is ~500 KB per pattern, but the code charges 256 KiB — so the charge is not conservative, it is an ~2x underestimate.

"100 distinct compiles consumed ~50 MB of real RSS" works out to ~500 KB per compiled Regex, for patterns that are single decimal digits. Meanwhile evaluator.rs:1705-1710 justifies entry_cost = REGEX_SIZE_LIMIT as "the conservative per-entry charge". If the 50 MB figure is accurate, the two claims contradict each other and the budget under-counts by roughly half.

The likely explanation is that size_limit bounds only the compiled program, while regex::Regex also lazily allocates a DFA cache on first use (bounded separately by RegexBuilder::dfa_size_limit, default 2 MB — untouched by this PR). If so, the real per-Regex ceiling is closer to 256 KiB + 2 MB than to 256 KiB, and an adversary can still drive ~8x the accounted memory by making every cached pattern actually match against a large haystack. Setting .dfa_size_limit(...) alongside .size_limit(...) would close that, and would make the flat entry_cost genuinely an upper bound.

Worth reconciling: either measure where the 50 MB actually goes and charge accordingly, or correct the comment. As written, a future reader will trust "conservative" and it is not.

Separately, per AGENTS.md the error-assertion standard for openjd-expr is message + expression source + caret. These four new failure tests assert only a substring ("exceeded limit (1000000 bytes)", "Invalid regex:" + "size limit"). The file already has assert_memory_exceeded (line 327) which at least also pins "Expression memory usage" — reusing it here would be closer to the standard, and regex_size_limit_rejects_large_nfa should assert the full message so a change in the regex crate wording is caught deliberately rather than silently.

[github-actions]

The "most templates use 1-3 distinct patterns" premise undercounts, because cache keys are not user patterns. re_match_fn (functions/regex.rs:539) keys on format!("^(?:{})", pat), while re_search/re_findall/re_sub/re_split key on the raw pat. So a template using pattern P with both re_match and re_search occupies two slots and pays two 256 KiB charges for one user-visible pattern.

That is fine on its own, but it means the effective cap for a template that mixes re_match with the other functions is closer to 16 distinct patterns than 32, and the accounted memory for such a template is double the real cost of the distinct patterns the author wrote. Worth reflecting in the constant doc comment so the 32 is not read as "32 patterns the template author wrote".

More broadly on the constant: 32 is a magic number with no with_* builder and no way for a caller to observe that they crossed it. When a template does cross it the only symptom is a silent performance cliff (recompile on every use) plus — per the charge issue on line 1722 — unbounded budget growth. Either exposing it as a builder knob alongside with_memory_limit/with_operation_limit, or scaling it off the memory limit (e.g. cache while regex_cache_bytes is under some fraction of memory_limit, which is the actual resource being protected), would avoid a fixed count that is simultaneously too small for mixed-function templates and unrelated to the budget it is meant to bound.

[github-actions]

regex_cache_bytes and current_memory can drift apart, and when they do this line silently discards the regex charge.

The invariant this line relies on is "any regex charge inside current_memory is also reflected in regex_cache_bytes". That holds for the loop body path, but not for the unresolved branch at line 1483-1517, which creates its child via child_evaluator (empty cache, regex_cache_bytes: 0) and then propagates back with absorb_counters only — absorb_counters copies current_memory but not regex_cache_bytes.

Nested comprehension over an unresolved inner iterable walks that path:

1. Outer iteration moves the cache into child and sets child.regex_cache_bytes.
2. child evaluates the inner comprehension, hits the unresolved branch, builds a grandchild with regex_cache_bytes: 0.
3. Grandchild compiles a regex: grandchild.current_memory += 262145, grandchild.regex_cache_bytes = 262145.
4. child.absorb_counters(&grandchild) copies the +262145 into child.current_memory, but child.regex_cache_bytes is untouched.
5. Back in the outer loop, self.absorb_counters(&child) picks up the inflated current_memory; self.regex_cache_bytes = child.regex_cache_bytes picks up no growth.
6. regex_delta computes to 0, and this line resets current_memory back to memory_baseline — dropping the charge entirely.

So the charge is dropped on exactly the nesting shape the PR is trying to bound. The simplest fix is to make absorb_counters propagate regex_cache_bytes too (it is described as "propagate resource counters back", and regex_cache_bytes is now one), and to move the cache into the unresolved-branch child the same way the loop body does — which would also mean the type-probe evaluation benefits from the cache instead of always recompiling.

More generally: tracking a subset of current_memory in a parallel field, and reconstructing current_memory by arithmetic on that subset, is fragile — every future site that touches one must remember the other. An alternative that avoids the invariant altogether is to keep regex charges out of current_memory and instead have check_memory/track compare against memory_limit - regex_cache_bytes, so the two never need to be reconciled.