Skip to content

test: property-check context-overflow classification and synthetic SSE#600

Open
ndycode wants to merge 2 commits into
mainfrom
claude/audit-81-context-overflow-property
Open

test: property-check context-overflow classification and synthetic SSE#600
ndycode wants to merge 2 commits into
mainfrom
claude/audit-81-context-overflow-property

Conversation

@ndycode

@ndycode ndycode commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds a fast-check property suite for lib/context-overflow.ts — the recovery path that turns "prompt too long" 400s into a synthetic SSE notice so the host session doesn't lock. The headline property is a round-trip oracle: the synthesizer's own output must parse as the Responses-API dialect and never re-classify as overflow.

What Changed

New test/property/context-overflow.property.test.ts (4 properties):

  1. Phrase contract — the seven documented overflow phrases classify at any casing and any position inside generated noise, but only behind the 400 status gate. The phrase list is pinned verbatim, so silently dropping a pattern from the SUT fails here.
  2. Negative space — noise-only bodies (hex + JSON punctuation, constructed so no pattern can occur) and empty bodies never classify, at any status 100–599.
  3. Round-trip oraclecreateContextOverflowResponse(model) returns a 200 with the synthetic headers, whose body parses line-by-line as valid SSE JSON in the Responses-API shape (response.created first, response.completed last, model echoed, the /compact notice in the terminal output payload — the recovery-01 dialect guarantee), and which is never re-classified as overflow by either the classifier or handleContextOverflow — the recovery path cannot recurse on its own output.
  4. Interception exactnesshandleContextOverflow handles exactly the classifier-positive 400s, and when it declines, the original response body remains readable (it reads a clone), so downstream error handling isn't starved.

No SUT bugs found.

Validation

  • npm test -- test/property/context-overflow.property.test.ts test/context-overflow.test.ts — 26/26 (new 4 + existing 22 untouched)
  • npm run typecheck (also via pre-commit hook)
  • npx eslint test/property/context-overflow.property.test.ts --max-warnings=0

Docs and Governance Checklist

  • Test-only; no behavior or docs surface changed

Risk and Rollback

  • Risk level: minimal — additive test file; conventions match the existing test/property/ suites.
  • Rollback plan: revert the single commit.

https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB


Generated by Claude Code

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

additive property test suite for lib/context-overflow.ts — four fast-check properties covering phrase classification, negative-space safety, SSE round-trip correctness, and interception exactness. no sut changes.

  • phrase contract + negative space (tests 1–2): arbNoise is restricted to hex chars + json punctuation, which provably excludes every overflow phrase, so false positives in the noise-only property are impossible by construction.
  • round-trip oracle (test 3): correctly async/awaits the asyncProperty; body-consumption ordering is safe because handleContextOverflow short-circuits on status !== 400 before any clone, so calling synthetic.text() first doesn't leave the response unusable.
  • interception exactness (test 4): accurately exercises the clone-not-consume contract; upstream.text() is only asserted in the decline path, consistent with the sut's response.clone().text() approach.

Confidence Score: 5/5

safe to merge — additive test file only, no sut changes, all four properties correctly awaited

single new test file with no production code changes. all async properties are properly awaited, the noise alphabet provably excludes every overflow phrase, body-consumption ordering is safe due to the status short-circuit in handleContextOverflow, and the round-trip oracle correctly validates the full SSE dialect contract including non-recursion. no regressions are possible from this change.

no files require special attention

Important Files Changed

Filename Overview
test/property/context-overflow.property.test.ts adds 4 fast-check property tests covering phrase contract, negative space, SSE round-trip oracle, and interception exactness; all four properties are correctly async-awaited, the noise alphabet provably excludes all overflow patterns, and the body-consumption ordering in the round-trip test is safe because the status guard short-circuits before any clone is attempted

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[property test invocation] --> B{test case}
    B --> C[test 1: phrase contract]
    B --> D[test 2: negative space]
    B --> E[test 3: round-trip oracle]
    B --> F[test 4: interception exactness]

    C --> C1[fc.constantFrom OVERFLOW_PATTERNS]
    C1 --> C2[arbCasing x3]
    C2 --> C3[arbNoise prefix + suffix]
    C3 --> C4[isContextOverflowError\nexpect true iff status=400]

    D --> D1[arbNoise only\nhex+JSON chars]
    D1 --> D2[fc.integer 100-599]
    D2 --> D3[isContextOverflowError\nexpect false always]

    E --> E1[createContextOverflowResponse model]
    E1 --> E2[parse SSE data lines]
    E2 --> E3{structure checks}
    E3 --> E4[payloads 0 = response.created]
    E3 --> E5[terminal = response.completed\nmodel echoed + /compact present]
    E3 --> E6[isContextOverflowError 200 body\nnever re-classifies]
    E3 --> E7[handleContextOverflow synthetic\nhandled=false, no recursion]

    F --> F1[new Response noise+pattern, status]
    F1 --> F2[handleContextOverflow]
    F2 --> F3{shouldHandle =\nstatus=400 && includePattern}
    F3 -->|true| F4[outcome.handled=true\nstatus=200 synthetic header]
    F3 -->|false| F5[outcome.handled=false\nupstream.text still readable]
Loading

Fix All in Codex

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
test/property/context-overflow.property.test.ts:119-126
**test 4 noise suffix gap**

the body construction in this test is always `noise + pattern` (prefix-only), while test 1 wraps the pattern with both a `before` and `after` noise segment. because `arbNoise` is hex + punctuation and none of the overflow patterns are constructable from that alphabet, this never causes a spurious failure — but it does mean fast-check shrinks toward the fully suffix-free case and any future pattern that starts with hex chars (e.g., hypothetical `"404 token exceeded"`) could silently make this test fragile. adding a suffix via a second `arbNoise` arbitrary would match the test-1 pattern and future-proof against SUT changes that expand the phrase list.

Reviews (2): Last reviewed commit: "test: await the async round-trip propert..." | Re-trigger Greptile

Four fast-check properties over the prompt-too-long recovery path:

- the seven documented overflow phrases classify at any casing and
  position, but only behind the 400 status gate (phrase list pinned
  verbatim so dropping one fails here)
- noise-only and empty bodies never classify at any status
- the synthetic response round-trips: 200 with the synthetic headers,
  parseable Responses-API SSE (created first, completed last, model
  echoed, /compact notice in the terminal output payload), and is
  never re-classified as overflow - the recovery path cannot recurse
  on its own output (recovery-01 dialect guard)
- handleContextOverflow intercepts exactly the classifier-positive
  400s, and a declined response's body stays readable (clone-read)

Companion to the property suites in #574/#575/#592-#599.

https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@ndycode, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 5 minutes and 26 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0d8e6e5d-c802-42c9-877b-5b18d1f89db6

📥 Commits

Reviewing files that changed from the base of the PR and between dae10cb and 203fd2b.

📒 Files selected for processing (1)
  • test/property/context-overflow.property.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/audit-81-context-overflow-property
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/audit-81-context-overflow-property

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread test/property/context-overflow.property.test.ts
Comment thread test/property/context-overflow.property.test.ts Outdated
…test

Greptile P1: the it callback was synchronous, so the asyncProperty's
promise was dropped and vitest greened the test before any assertion
ran. Same pattern as the fourth test now.

https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
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