fix(arx): account for chat-safe transport when selecting ARX wire form - #15
Conversation
Deploying agent-render with
|
| Latest commit: |
933dbf7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6e559577.agent-render.pages.dev |
| Branch Preview URL: | https://codex-fix-base64url-candidat.agent-render.pages.dev |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Overview
AnalysisThe PR correctly implements chat-safe transport length calculation for ARX wire form selection:
The code handles edge cases correctly:
Files Reviewed (2 files)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 933dbf7bc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const cp = value.codePointAt(i)!; | ||
| if (cp < 128) { | ||
| len += 1; | ||
| len += CHAT_SAFE_ASCII_FRAGMENT_CHARS.test(value[i]) ? 1 : 3; |
There was a problem hiding this comment.
B. fragments over valid base76
This 3x penalty means encodeEnvelopeAsync(..., { codec: "arx" }) can now pick the chat-safe B. wire form even when it makes the real fragment exceed the 8,000-character limit that callers still enforce with hash.length - 1 (for example in createGeneratedArtifactLinkAsync). I reproduced this with a packed markdown payload of about 5,900 repeated bullet lines: the base76 fragment is 7,985 chars and would still work, but its computed transport length becomes 10,159 here, so the encoder instead selects an 8,331-char B. fragment and link creation rejects it. That is a user-visible regression for large payloads near the fragment budget.
Useful? React with 👍 / 👎.
The old @git-diff-view/react output read as a debug dump with pseudo-table headers; the decided direction (cursor session, criticism #15) was to hand rendering to diffs.com's library. @pierre/diffs is Shiki-based, renders into shadow DOM, and computes before/after diffs from raw contents, which the old stack could not do (the oldContent/newContent path rendered an empty diff body). - diff-renderer.tsx: PatchDiff per parsed patch file and MultiFileDiff for before/after contents, imported through a small use-client bridge (src/lib/diff/pierre-react.ts) so the deferred chunk id stays stable across webpack graphs. The parse gate, per-file nav, binary handling, unified/split toggle, fallback, error boundary, and every data attribute are unchanged. - The vendored stylesheet pipeline is gone: diff-view-stylesheet.ts, public/vendor/diff-view-pure.css(.br), the mirror step in assets:compress, its _headers entry, and the sync test. Shadow-DOM styles are themed via --diffs-* custom properties on .diff-renderer-frame, keeping diff bodies dark charcoal in both themes. - Budget re-keyed to the artifact-stage dynamic import and tightened to 160 KiB gzipped (measures 142 KiB; Shiki grammars load as separate on-demand chunks outside this key, unlike the old bundled highlighter at 323.9/340 KiB). - Header tests keep exercising generic *.css.br serving via a neutral fixture name; the e2e stylesheet-lifecycle test now asserts shadow-DOM rendering with no external diff stylesheet. Lint, typecheck, 321 unit tests, build budgets, and the chromium e2e suite (28/28) pass. Visual snapshots are not regenerated here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The old @git-diff-view/react output read as a debug dump with pseudo-table headers; the decided direction (cursor session, criticism #15) was to hand rendering to diffs.com's library. @pierre/diffs is Shiki-based, renders into shadow DOM, and computes before/after diffs from raw contents, which the old stack could not do (the oldContent/newContent path rendered an empty diff body). - diff-renderer.tsx: PatchDiff per parsed patch file and MultiFileDiff for before/after contents, imported through a small use-client bridge (src/lib/diff/pierre-react.ts) so the deferred chunk id stays stable across webpack graphs. The parse gate, per-file nav, binary handling, unified/split toggle, fallback, error boundary, and every data attribute are unchanged. - The vendored stylesheet pipeline is gone: diff-view-stylesheet.ts, public/vendor/diff-view-pure.css(.br), the mirror step in assets:compress, its _headers entry, and the sync test. Shadow-DOM styles are themed via --diffs-* custom properties on .diff-renderer-frame, keeping diff bodies dark charcoal in both themes. - Budget re-keyed to the artifact-stage dynamic import and tightened to 160 KiB gzipped (measures 142 KiB; Shiki grammars load as separate on-demand chunks outside this key, unlike the old bundled highlighter at 323.9/340 KiB). - Header tests keep exercising generic *.css.br serving via a neutral fixture name; the e2e stylesheet-lifecycle test now asserts shadow-DOM rendering with no external diff stylesheet. Lint, typecheck, 321 unit tests, build budgets, and the chromium e2e suite (28/28) pass. Visual snapshots are not regenerated here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The old @git-diff-view/react output read as a debug dump with pseudo-table headers; the decided direction (cursor session, criticism #15) was to hand rendering to diffs.com's library. @pierre/diffs is Shiki-based, renders into shadow DOM, and computes before/after diffs from raw contents, which the old stack could not do (the oldContent/newContent path rendered an empty diff body). - diff-renderer.tsx: PatchDiff per parsed patch file and MultiFileDiff for before/after contents, imported through a small use-client bridge (src/lib/diff/pierre-react.ts) so the deferred chunk id stays stable across webpack graphs. The parse gate, per-file nav, binary handling, unified/split toggle, fallback, error boundary, and every data attribute are unchanged. - The vendored stylesheet pipeline is gone: diff-view-stylesheet.ts, public/vendor/diff-view-pure.css(.br), the mirror step in assets:compress, its _headers entry, and the sync test. Shadow-DOM styles are themed via --diffs-* custom properties on .diff-renderer-frame, keeping diff bodies dark charcoal in both themes. - Budget re-keyed to the artifact-stage dynamic import and tightened to 160 KiB gzipped (measures 142 KiB; Shiki grammars load as separate on-demand chunks outside this key, unlike the old bundled highlighter at 323.9/340 KiB). - Header tests keep exercising generic *.css.br serving via a neutral fixture name; the e2e stylesheet-lifecycle test now asserts shadow-DOM rendering with no external diff stylesheet. Lint, typecheck, 321 unit tests, build budgets, and the chromium e2e suite (28/28) pass. Visual snapshots are not regenerated here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Motivation
arxcandidates never chose the new chat-safe base64url (B.) wire form becausecomputeTransportLengthtreated all ASCII characters as equal length, letting punctuation-heavy base76 always win.Description
computeTransportLengthinsrc/lib/payload/fragment.tsto conservatively count ASCII punctuation outside a chat-safe unreserved subset as escape-prone so those chars are treated like percent-escaped bytes instead of single characters.CHAT_SAFE_ASCII_FRAGMENT_CHARSregex and use it during transport-length computation so dense ASCII-safe encodings (like base64urlB.) can win when appropriate.tests/arx-codec.test.tsthat assertsencodeEnvelopeAsync(..., { codec: "arx" })can emit thev1.arx.<dictVersion>.B.base64url fragment in auto mode.Testing
npm test -- --run tests/arx-codec.test.tsand the test file passed (all tests succeeded, including the newB.selection test).npm run typecheckandtsccompleted without errors.npm run lintand the lint and public export docs checks passed.Codex Task