Skip to content

feat(tier3): decouple Tier-3 resource bound from the review cap (Phase 1) - #98

Open
hiskudin wants to merge 2 commits into
mainfrom
hiskias/tier3-resource-bound
Open

hiskudin wants to merge 2 commits into
mainfrom
hiskias/tier3-resource-bound

Conversation

@hiskudin

@hiskudin hiskudin commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Phase 1 — decouple Tier-3's resource bound from the review cap

Problem

tier3.maxTextLength (default 10,000) did double duty: (a) the LLM review window, and (b) the only guard on how much the Tier-3 serializer traverses/allocates. Tier 1/2 already have a decoupled resource bound (traversal.maxSize = 10 MB, via estimatedBytes/shouldContinueTraversal); Tier-3 had none. Verified on main, three unbounded input traversals:

  • sumInputStringChars(value) — full payload walk on every tier3_only call (dominant, always-on).
  • The array all-scalar scan (for (const x of v)) — O(array) to decide the [N numbers] summary.
  • tier3SpreadOrder(n) + Array.from({length:n}) + the final .filter — O(n) allocations for large lists.

So Tier-3 was on a strictly worse DoS footing than the other tiers.

Change (no API change)

Give the Tier-3 traversal a byte budget = traversal.maxSize (10 MB), metered with the shared estimateSize mechanism, decoupled from maxTextLength (now the review window only):

  • Meter serialize (per node — estimateSize returns an array/object's breadth, so a huge one trips in O(1)) and sumStringContent/sumInputStringChars; stop past the budget. Reset before each traversal so each gets a fresh maxSize (matching Tier 1/2's single-pass bound); sizeLimitHit OR-accumulates across passes.
  • Cap tier3SpreadOrder output (bounded even-spread sample for huge n, no O(n) scratch); drop the greedy O(n) order array / new Array(n) / .filter (collect + sort, O(emitted)).
  • Hitting the resource bound flags oversize (budgetExceeded) → existing onOversize governs it (skip allows + flags coverageDegraded; block/scan_anyway fail closed) — same contract as ceiling overflow. Not an unconditional block.

Decisions (from the design note + measured numbers)

  • Global ceiling 10 MB, metered on inner-body bytes (p99 real inner body = 672 KB → 10 MB ≈ 15× over). Resource ceiling is global (shared by all tiers); per-tier work caps stay separate.
  • Resource-bound hit = oversize (policy-governed), not a hard block — keeps the blockHighRisk/onOversize contract intact.

Tests

  • Resource bound flags oversize on a large low-string-content payload (a numeric array that collapses to a summary — the emitted < input string check would NOT catch it), across skip/block/scan_anyway.
  • A payload within the bound is not falsely flagged (no over-block). Full suite: 439 pass; tsc/biome clean.

Out of scope

Phase 2 (evolve the Tier3Provider contract so the provider owns window truncation; demote maxTextLength to a convenience default) — gated on the Tier-3 extraction/as-a-service decision.

🤖 Generated with Claude Code


Adversarial review addressed

  • (HIGH, fixed) The array/object element loops broke only on the char budget, not the byte meter — a mid-collection meter trip still cost O(remaining length) of loop iterations (~345ms on a 3M-element array). Now break on meter.hit too.
  • (LOW, fixed) tier3SpreadOrder maxOut<=1 edge; comment accuracy on estimateSize (O(1) arrays / O(keys) objects).
  • Verified clean: collect-then-sort preserves order/content; no false-oversize on benign payloads; meter-trip state coherent; cascade unaffected.

Bound accuracy (worth noting)

The byte meter is reset per traversal (greedy serialize, reserve serialize, input-sum), each bounded by maxSize. A char-oversize payload (common — any list past the ~47.5 KB chunk ceiling) runs the reserve pass too, so worst case a single call does up to ~3 metered traversals (≈3× maxSize = ~30 MB of work), not one. Still a bounded constant; resetting avoids double-counting the same nodes across the greedy/reserve passes (which would false-trip benign payloads).

…e 1)

tier3MaxTextLength did double duty — the LLM review window AND the only guard
on how much the Tier-3 serializer traverses/allocates. Unlike Tier 1/2 (bound
by traversal.maxSize = 10MB via estimatedBytes/shouldContinueTraversal),
Tier-3 had no resource bound: sumInputStringChars walked the whole payload on
every call, the array all-scalar scan was O(array), and tier3SpreadOrder(n)
allocated O(n) — a DoS footing worse than the other tiers.

Give the Tier-3 traversal its own byte budget = traversal.maxSize (10MB),
metered with the shared estimateSize mechanism and decoupled from
tier3MaxTextLength (now the review window only):
- Meter serialize (per node; an array/object returns its breadth so a huge
  one trips in O(1)) and sumStringContent/sumInputStringChars; stop past the
  budget. Reset per traversal so each gets a fresh maxSize (matches Tier 1/2's
  single-pass bound); sizeLimitHit OR-accumulates.
- Cap tier3SpreadOrder output (bounded even-spread sample for huge n; no O(n)
  scratch); drop the greedy O(n) order/`new Array(n)`/filter (collect+sort,
  O(emitted)).
- Hitting the resource bound flags oversize (budgetExceeded) → onOversize
  governs it (skip allows+flags, block/scan_anyway fail closed) — same
  contract as ceiling overflow. No API change.

Tests: resource bound flags oversize on large low-string-content bulk (which
the string check wouldn't catch); a within-budget payload is not falsely
flagged. 439 pass.

Phase 2 (move the review cap into the Tier3Provider contract) is out of scope
— gated on the Tier-3 extraction/as-a-service decision.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@hiskudin
hiskudin requested a review from a team as a code owner September 25, 2026 10:56
Copilot AI lite review requested due to automatic review settings September 25, 2026 10:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…n the byte meter

Adversarial review of PR #98 found:
- (HIGH, reproduced) The array/object element loops broke only on the char
  budget (`used >= outerCap`), not the byte meter. When the meter tripped
  mid-collection, serialize() bailed per call but the loop still ran
  O(remaining length) — ~345ms of wasted iteration on a 3M-element array vs
  ~0.1ms once fixed. Add `meter.hit` to both loop breaks (matching runRecords).
- (LOW) tier3SpreadOrder returned >maxOut indices when maxOut<=1 (unreachable
  in realistic configs); clamp + slice to honor the cap.
- (LOW) corrected the serialize meter comment (estimateSize is O(1) for
  arrays, O(keys) for objects, not O(1) for both).

Clean per the review: the collect-then-sort refactor preserves order/content,
no false-oversize on benign payloads, meter-trip state coherent, cascade
unaffected. Regression test: a mid-array byte-budget trip flags oversize. 440 pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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