ARX4: context-mixing codec, 10% smaller fragments, markdown-link fix - #106
Conversation
URL serializers percent-encode non-ASCII fragment characters, so a baseBMP arx3 fragment triples in size the moment it is embedded in a markdown link destination (76 visible chars became a 704 char link). Markdown links now carry a transport-budgeted fragment of the same payload: encoding gains a budgetByTransport option that measures the arx3 baseBMP candidate by percent-escaped transport length, letting an ASCII wire win for that surface. The paste URL keeps the dense baseBMP form, and both fragments decode to the same envelope. The visible-length policy for the primary URL is unchanged; the new option is a documented per-surface exception beside the policy note. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
arx4 keeps the arx3 tuple, substitution, and dictionary stages and replaces brotli with a deterministic integer context mixer including a column-position context for tables. Coded output is ~5% smaller than arx3 on the sample fixtures (CSV improves the most), and the mixer is bit-exact across JS engines because the coding path is integer-only. Wire format: tag e, then one prior-id char (m/c/j/s/n) naming the priming corpus, then the usual four wire alphabets. All prior ids currently resolve to the same dictionary-derived text; the per-kind ids are reserved so curated priors can land later without a wire change. arx4 leads the async codec priority and auto-selection falls back to arx3 where arx4 loses. Encode is roughly 100x slower than brotli (hundreds of ms on large artifacts); the arx family is already async-only, which absorbs that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The arx4 mixer now primes from public/arx4-priors.json, a lazy-loaded asset carrying three curated kind corpora (45 KB raw, 13 KB brotli). The shared 2,203-char prefix stays derived from the dictionaries at runtime, so nothing is duplicated. With the asset active the sample fixtures code 10.3% smaller than arx3 (up from 5.2% asset-less), and all seven now beat arx3. Loading follows the dictionary pattern: version pin refusing newer, retry on transient failure, sync inject hook for tests. Encode degrades to the shared prior when the asset is unavailable so link creation never blocks; decoding a fragment that names a curated prior without the asset throws a retryable error instead of mis-decoding. A new Playwright spec proves the codec byte-exact across engines: the same pinned vectors (now shared between unit and e2e suites) encode identically under Chromium and WebKit and round-trip through the real app UI. scripts/build-arx4-priors.mjs regenerates the asset and hard-fails unless the reassembled priors sha256-match the benched construction. Chromium visual baselines are regenerated for the codec chip row added with arx4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rior Decode was loading the priors asset for every arx4 fragment, but s and n fragments never use it. The prior id is the first payload char, so decode now checks it before fetching, and a test pins that s and n fragments decode with fetch stubbed to fail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughARX4 adds deterministic context-mixing compression with curated priors. It integrates with asynchronous fragment selection, generated links, URL budgeting, static assets, protocol documentation, and unit, browser, and determinism tests. ChangesARX4 codec and shared wire contracts
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Deploying agent-render with
|
| Latest commit: |
f41c294
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://18514a16.agent-render.pages.dev |
| Branch Preview URL: | https://arx4-experimentation.agent-render.pages.dev |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
| Filename | Overview |
|---|---|
| src/lib/payload/arx4-codec.ts | Implements deterministic ARX4 compression, curated-prior loading, and integrity checks. |
| src/lib/payload/fragment-arx.ts | Integrates ARX4 candidate generation and decoding, but its dictionary compatibility guard does not establish exact dictionary identity. |
| src/lib/payload/fragment.ts | Adds ARX4 selection and derives both sharing surfaces from one candidate pool. |
| src/lib/payload/link-creator.ts | Uses the shared candidate pool to generate transport-appropriate paste and markdown URLs without recompressing. |
| tests/arx4-dictionary-pin-guard.test.ts | Covers fallback and version-skew states but not same-version dictionary-content divergence. |
| tests/link-creator-encode-once.test.ts | Verifies one ARX4 compression pass and equivalent decoding across both generated surfaces. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
E[Artifact envelope] --> C[Build codec candidates once]
C --> P[Default visible-length selection]
C --> M[URL-serialized selection]
P --> U[Copy-paste URL]
M --> L[Markdown link]
Comments Outside Diff (1)
-
src/lib/payload/fragment-arx.ts, line 132-137 (link)Same-version dictionaries bypass pinning
When a CDN or self-hosted deployment serves a structurally valid but mismatched base or overlay dictionary with the expected version number,
arx4DictionariesMatchPinsaccepts it even though its contents alter substitution and mixer initialization. The encoder can then mint anefragment that viewers using the shipped dictionaries cannot decode.Context Used: AGENTS.md (source)
Reviews (8): Last reviewed commit: "Keep trying the remaining priors URLs af..." | Re-trigger Greptile
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Incremental review of commit Verified in the changed code:
Files Reviewed (2 files)
Previous Review Summaries (7 snapshots, latest commit a297434)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a297434)Status: 1 Issue Found | Recommendation: Address before merge Overview
Incremental review of commit Verified in the changed code:
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Files Reviewed (14 files)
Previous review (commit 1c763ad)Status: No Issues Found | Recommendation: Merge Incremental review of commit Verified in the changed code:
This incremental diff also resolves the previously-raised finding about URL-based path parsing on this script. No new issues were introduced, and no other previously-open inline comments fell on changed lines. Files Reviewed (1 file)
Previous review (commit 1069bda)Status: No Issues Found | Recommendation: Merge Incremental review of commit Verified in the changed code:
No new issues were introduced by this incremental diff, and no previously-open inline comments fell on changed lines. Previous review (commit 36c9794)Status: No Issues Found | Recommendation: Merge Incremental review of commit Verified in the changed code:
No new issues were introduced by this incremental diff, and no previously-open inline comments fell on changed lines. Previous review (commit 827c835)Status: No Issues Found | Recommendation: Merge Incremental review of commit Verified in the changed code:
All pre-existing inline comments on changed files ( Previous review (commit 9725021)Status: No Issues Found | Recommendation: Merge Incremental review of commit Verified:
The two prior inline findings sit on unchanged files ( Files Reviewed (2 files)
Previous review (commit 2cd1e15)Status: No Issues Found | Recommendation: Merge The arx4 context-mixing codec and its integration are well-constructed. Verified the load-bearing determinism invariants:
One non-blocking note (not flagged inline): Files Reviewed (10 code files)
Reviewed by Kimi-K3 · Input: 30.7K · Output: 2.6K · Cached: 109.1K |
The frozen benchmark script the priors asset is built from is maintainer-local research material, not repo content. The build script now takes its path as an argument, and asset integrity continues to rest on the sha256 pins in tests/arx4-priors.test.ts, which need only the shipped asset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2cd1e15 to
9725021
Compare
![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: 2cd1e153ac
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/payload/link-creator.ts (1)
193-199: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftarx4's coding pass is one long synchronous block on the interactive path, and link creation runs it twice. The shared root cause is that
arx4CompressEnvelopenever yields (~770 ms per 60 KB per this codec's own header, plus a ~16 KB priming pass), while the async signatures around it suggest otherwise; encoding the same envelope a second time only to change candidate scoring doubles that stall.
src/lib/payload/link-creator.ts#L193-L199: build the candidate set once and run selection twice (default scoring andbudgetByTransport) instead of callingencodeEnvelopeAsynctwice.src/lib/payload/fragment-arx.ts#L312-L322: move the arx4 coding pass off the main thread (worker) or chunk the byte loop with yields, so the codec's async surface actually keeps the UI responsive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/payload/link-creator.ts` around lines 193 - 199, In src/lib/payload/link-creator.ts lines 193-199, encode the normalized envelope once to build the candidate set, then run selection with default scoring and budgetByTransport scoring instead of calling encodeEnvelopeAsync twice. In src/lib/payload/fragment-arx.ts lines 312-322, update arx4CompressEnvelope so its coding pass yields during processing, either by moving it to a worker or chunking the byte loop, while preserving the existing async behavior and output.
🧹 Nitpick comments (8)
scripts/bench-arx4-zstd-dict.mjs (1)
513-519: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
small[0]/small[1]positionally decode named fixtures. The prose labels themjson-packageandsmall-markdown, which only holds whilefixtureskeeps its current order. Looking them up by name would make the report immune to reordering.Also applies to: 571-571
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/bench-arx4-zstd-dict.mjs` around lines 513 - 519, Update the report generation around the small fixture selection and its later uses to look up “json-package” and “small-markdown” by name rather than relying on small[0]/small[1] positions. Reuse the existing result names and preserve the current labels and metrics output even when fixtures are reordered.scripts/bench-arx4-cm.mjs (1)
2581-2616: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
--reuse-timingsthrows if the report is absent.readFileSync(REPORT_PATH)runs before any work; a missing prior report yields a raw ENOENT rather than a message pointing at the documented two-pass flow. Cheap guard, only affects a debug flag.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/bench-arx4-cm.mjs` around lines 2581 - 2616, Update readTimingOverrides to handle a missing REPORT_PATH before calling readFileSync. When --reuse-timings is requested without an existing report, emit a clear message directing the user to run the documented initial pass, then exit or otherwise stop before parsing; preserve the existing override parsing for available reports.scripts/bench-arx4-token.mjs (1)
1589-1597: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComparison bars are hardcoded literals.
−5.2%and−11.6%are transcribed from the CM bench rather than derived, so they silently go stale whendocs/arx4-cm-bench.mdis regenerated. Consider naming them as constants near the top with a comment citing the source report.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/bench-arx4-token.mjs` around lines 1589 - 1597, Define named constants near the top for the CM corpus and key-fixture comparison thresholds, documenting that their values come from docs/arx4-cm-bench.md. Replace the hardcoded -5.2 and -11.6 literals in the summary comparisons and displayed labels with those constants, preserving the existing formatting and comparison behavior.scripts/bench-arx4-blind.mjs (2)
54-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOrder the sentinel check before using
start. WhenstartMarkeris absent,source.indexOf(endMarker, start + startMarker.length)still runs with a bogus offset before the guard fires; moving thestart === -1check up makes the failure path explicit.♻️ Reorder guards
function extractMarkedBlock(source, startMarker, endMarker, label) { const start = source.indexOf(startMarker); + if (start === -1) throw new Error(`could not extract ${label} from frozen source`); const end = source.indexOf(endMarker, start + startMarker.length); - if (start === -1 || end === -1 || end <= start) { + if (end === -1 || end <= start) { throw new Error(`could not extract ${label} from frozen source`); } return source.slice(start, end); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/bench-arx4-blind.mjs` around lines 54 - 61, Update extractMarkedBlock to check whether start is -1 immediately after locating startMarker, before calculating the end-marker search offset. Only search for endMarker after a valid start, then retain the existing validation and error behavior for missing or invalid markers.
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorpus lives at a hardcoded
/tmppath that is not in the repository. Anyone re-running this script gets an ENOENT with no hint about the expected corpus format or provenance. Consider accepting an--corpusargument (defaulting to this path) and emitting an actionable error when the file is missing.Also applies to: 522-522
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/bench-arx4-blind.mjs` at line 17, Update the benchmark script’s corpus-path handling around CORPUS_PATH to accept an optional --corpus command-line argument while retaining the current /tmp path as the default. When the corpus file is missing, emit an actionable error that identifies the expected path and explains the required corpus format or provenance before exiting.scripts/arx4-lm-oracle.py (1)
309-322: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse a base-model accessor instead of hard-coding
.model. Causal-LM checkpoints can expose the transformer as.transformer,.backbone, or another attribute, so adding one toMODEL_PATHSmay raiseAttributeErrorduring scoring. Prefer an accessor that matches the public embedding call, or handle the model-specific attribute with a clear error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/arx4-lm-oracle.py` around lines 309 - 322, The scoring path around the direct model.model call must use a supported base-model accessor instead of hard-coding the .model attribute. Reuse the accessor compatible with get_output_embeddings, or resolve the model-specific transformer/backbone attribute and raise a clear error when none is available, while preserving the existing hidden-state scoring flow.scripts/arx4-cm-determinism.mjs (1)
2-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe frozen source is cited as a
/tmppath, which is not reproducible.Both the header comment and the generated report point at
/tmp/bench-arx4-cm-final-snapshot.mjslines 1915-2347. A reader cannot re-derive or re-verify the extraction. Reference a committed artifact (or record its SHA-256, asdocs/arx4-blind-ood.mddoes for the scripts) instead.Also applies to: 2428-2429
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/arx4-cm-determinism.mjs` around lines 2 - 7, The determinism checker references a non-reproducible /tmp source path in its header and generated report. Update the relevant comments and report-generation logic in scripts/arx4-cm-determinism.mjs to reference a committed artifact or include its SHA-256, matching the reproducibility convention used by docs/arx4-blind-ood.md; remove the /tmp path and preserve the existing source-range context where applicable.public/arx4-priors.json.br (1)
1-1: 🗄️ Data Integrity & Integration | 🔵 TrivialGuard
.jsonand.brfrom diverging in CI.
public/arx4-priors.json.bris generated byscripts/build-arx4-priors.mjs, and the codec prefers it via/arx4-priors.json.brbefore falling back to/arx4-priors.json. Since CI does not currently regenerate/verify this asset, add a checked script that re-runs the builder and fails when the worktree is dirty so a future edit doesn’t leave browsers using an older priors corpus while Node uses the updated one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/arx4-priors.json.br` at line 1, Add a CI-checked validation step for the generated public/arx4-priors.json.br asset using scripts/build-arx4-priors.mjs: rerun the builder and fail if it leaves the worktree dirty, ensuring the compressed asset stays synchronized with public/arx4-priors.json and preventing stale browser data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/arx4-cm-bench.md`:
- Line 65: Update the cmRawPrimedKind description to remove the inaccessible
origin/cursor branch reference and align the kind tags with the shipped
specification in docs/payload-format.md: use m, c, j, s, and n, stating that
these shipped IDs are authoritative.
In `@docs/payload-format.md`:
- Line 31: The arx4 format description must define the `n` prior alongside the
existing `m`, `c`, `j`, and `s` prior behavior. Update the arx4 entry to state
what corpus or priming behavior `n` represents, ensuring the documented prior
set matches the decoder’s recognized IDs and preserves the hard-error behavior
for unknown IDs.
In `@scripts/arx4-cm-determinism.mjs`:
- Around line 2180-2203: Update buildExactPrior to return the exact Buffer slice
from available.subarray(0, targetBytes) instead of converting it to UTF-8 text
and back. Adjust sharedPrior and each kindPriors entry to retain that Buffer
directly, while preserving the existing labels, corpus selection, and target
byte sizes so priming uses exactly targetBytes.
- Around line 2307-2347: Update testBrowser’s browser setup around
browser.newPage() and page.addScriptTag() to catch setup failures and record
them as a blocked engine result, rather than allowing them to escape. Preserve
the existing per-test error reporting, ensure browser cleanup still runs via the
finally block, and return the engine so the report and normal exit path can
proceed with an explicit blocker row.
In `@scripts/arx4-discord-paste-test.mjs`:
- Around line 82-83: Update the clipboard verification around
spawnSync("pbpaste", ...) so it handles null or unavailable paste.stdout before
calling toString. Preserve the intended self-check failure reporting for missing
binaries or signal-killed processes instead of allowing a TypeError from
clipboard conversion.
In `@scripts/arx4-lm-oracle.py`:
- Around line 29-42: The evaluation scripts use machine-specific absolute paths,
preventing reproducible local and CI runs. In scripts/arx4-lm-oracle.py lines
29-42, parameterize CORPUS_PATH, BASELINE_REPORT_PATH, OUTPUT_PATH, and
MODEL_PATHS through CLI arguments or environment variables, with report and
output defaults repository-relative. In scripts/bench-arx4-blind.mjs line 17,
accept the corpus location as an argument defaulting to
/tmp/artifacts-corpus.md, and fail with a message describing the required
===ARX4-ARTIFACT-BEGIN format when the corpus is missing.
In `@scripts/bench-arx4-cm.mjs`:
- Around line 1868-1873: Validate that curatedMarkdownSections,
curatedCodeSections, and curatedJsonSections have identical lengths before the
flatMap zip is performed. In the curated corpus construction around
curatedSections, fail fast with a clear error when the lengths differ, and only
build curatedCorpusText after validation so missing entries cannot be coerced
into blank separators.
In `@scripts/bench-arx4-token.mjs`:
- Around line 913-926: Update normalizeCounts to detect when halving can no
longer reduce the total because every normalized count is already 1, then exit
the loop with a clear diagnostic instead of retrying forever. Preserve the
existing normalization behavior while the total can still decrease, and ensure
the guard handles vocabularies whose size exceeds PRIME_TOTAL_LIMIT.
In `@scripts/bench-arx4-zstd-dict.mjs`:
- Around line 491-494: Extend the validation around the frame measurement
consistency checks to assert the cross-key equalities required by the
frame-accounting prose: dictId16 must equal dictId64 and dictIdRawTuple16, and
core16 must equal core64. Apply the same validation to the corresponding later
accounting path near the second referenced usage, and throw a clear error when
any equality fails.
- Around line 16-18: Replace the hardcoded ZSTD, BROTLI, and XZ paths with
environment-configurable binary paths, falling back to resolving each executable
through PATH. Update the spawnSync calls using these constants so the benchmark
works across macOS architectures and Linux.
In `@scripts/build-arx4-priors.mjs`:
- Around line 98-111: Replace the tautological sha256 comparison in the
prior-validation flow with comparisons against pinned expected digests for each
prior or its reconstructed corpus. Update the relevant frozen-script and
dictionary validation symbols so changes to either the shipped dictionaries or
frozen corpus fail the build, while preserving the existing prefix and
byte-length checks.
In `@src/lib/payload/arx4-codec.ts`:
- Around line 762-794: Update isArx4Priors to require asset.version to be a
finite, non-negative integer before accepting the asset. Keep installArx4Priors
as the single installation gate so invalid versions return false and never
update priorsSlot or reach the coder.
- Around line 777-785: Update fetchArx4Priors to enforce a finite request
timeout, using an abort signal or equivalent mechanism passed to fetch(url).
Ensure timeout failures are caught by the existing fallback path and return
null, allowing ensureArx4PriorsLoaded and buildArx4Candidates to proceed without
hanging.
In `@src/lib/payload/fragment-arx.ts`:
- Around line 155-185: Make the encode-side arx4 priors path non-fatal for
forward-incompatible versions: update ensureArx4PriorsLoaded and its
buildArx4Candidates caller to avoid propagating
assertArx4PriorsNotNewerThanExpected failures during candidate construction,
falling back to the s prior or omitting arx4 candidates. Preserve the existing
hard refusal for decode paths and keep other codec candidates buildable when
arx4 priors are unavailable or newer than expected.
---
Outside diff comments:
In `@src/lib/payload/link-creator.ts`:
- Around line 193-199: In src/lib/payload/link-creator.ts lines 193-199, encode
the normalized envelope once to build the candidate set, then run selection with
default scoring and budgetByTransport scoring instead of calling
encodeEnvelopeAsync twice. In src/lib/payload/fragment-arx.ts lines 312-322,
update arx4CompressEnvelope so its coding pass yields during processing, either
by moving it to a worker or chunking the byte loop, while preserving the
existing async behavior and output.
---
Nitpick comments:
In `@public/arx4-priors.json.br`:
- Line 1: Add a CI-checked validation step for the generated
public/arx4-priors.json.br asset using scripts/build-arx4-priors.mjs: rerun the
builder and fail if it leaves the worktree dirty, ensuring the compressed asset
stays synchronized with public/arx4-priors.json and preventing stale browser
data.
In `@scripts/arx4-cm-determinism.mjs`:
- Around line 2-7: The determinism checker references a non-reproducible /tmp
source path in its header and generated report. Update the relevant comments and
report-generation logic in scripts/arx4-cm-determinism.mjs to reference a
committed artifact or include its SHA-256, matching the reproducibility
convention used by docs/arx4-blind-ood.md; remove the /tmp path and preserve the
existing source-range context where applicable.
In `@scripts/arx4-lm-oracle.py`:
- Around line 309-322: The scoring path around the direct model.model call must
use a supported base-model accessor instead of hard-coding the .model attribute.
Reuse the accessor compatible with get_output_embeddings, or resolve the
model-specific transformer/backbone attribute and raise a clear error when none
is available, while preserving the existing hidden-state scoring flow.
In `@scripts/bench-arx4-blind.mjs`:
- Around line 54-61: Update extractMarkedBlock to check whether start is -1
immediately after locating startMarker, before calculating the end-marker search
offset. Only search for endMarker after a valid start, then retain the existing
validation and error behavior for missing or invalid markers.
- Line 17: Update the benchmark script’s corpus-path handling around CORPUS_PATH
to accept an optional --corpus command-line argument while retaining the current
/tmp path as the default. When the corpus file is missing, emit an actionable
error that identifies the expected path and explains the required corpus format
or provenance before exiting.
In `@scripts/bench-arx4-cm.mjs`:
- Around line 2581-2616: Update readTimingOverrides to handle a missing
REPORT_PATH before calling readFileSync. When --reuse-timings is requested
without an existing report, emit a clear message directing the user to run the
documented initial pass, then exit or otherwise stop before parsing; preserve
the existing override parsing for available reports.
In `@scripts/bench-arx4-token.mjs`:
- Around line 1589-1597: Define named constants near the top for the CM corpus
and key-fixture comparison thresholds, documenting that their values come from
docs/arx4-cm-bench.md. Replace the hardcoded -5.2 and -11.6 literals in the
summary comparisons and displayed labels with those constants, preserving the
existing formatting and comparison behavior.
In `@scripts/bench-arx4-zstd-dict.mjs`:
- Around line 513-519: Update the report generation around the small fixture
selection and its later uses to look up “json-package” and “small-markdown” by
name rather than relying on small[0]/small[1] positions. Reuse the existing
result names and preserve the current labels and metrics output even when
fixtures are reordered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 35b33d16-4894-4dde-8d95-3deea7cf7f34
⛔ Files ignored due to path filters (8)
package-lock.jsonis excluded by!**/package-lock.jsontests/e2e/visual.spec.ts-snapshots/code-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/csv-compact-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/diff-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/empty-state-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/json-light-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/markdown-dark-chromium.pngis excluded by!**/*.pngtests/e2e/visual.spec.ts-snapshots/markdown-light-chromium.pngis excluded by!**/*.png
📒 Files selected for processing (39)
README.mddocs/architecture.mddocs/arx4-blind-ood.mddocs/arx4-cm-bench.mddocs/arx4-cm-determinism.mddocs/arx4-discord-counting.mddocs/arx4-lm-oracle.mddocs/arx4-token-bench.mddocs/arx4-zstd-dict-bench.mddocs/payload-format.mddocs/url-fragments.mdpackage.jsonplaywright.config.tspublic/_headerspublic/arx4-priors.jsonpublic/arx4-priors.json.brscripts/arx4-cm-determinism.mjsscripts/arx4-discord-paste-test.mjsscripts/arx4-lm-oracle.pyscripts/bench-arx4-blind.mjsscripts/bench-arx4-cm.mjsscripts/bench-arx4-token.mjsscripts/bench-arx4-zstd-dict.mjsscripts/build-arx4-priors.mjssrc/lib/payload/arx-codec.tssrc/lib/payload/arx4-codec.tssrc/lib/payload/fragment-arx.tssrc/lib/payload/fragment.tssrc/lib/payload/link-creator.tssrc/lib/payload/schema.tstests/arx-codec.test.tstests/arx4-codec.test.tstests/arx4-priors.test.tstests/compact-header.test.tstests/components/link-creator.test.tsxtests/e2e/arx4-determinism.spec.tstests/fixtures/arx4-vectors.tstests/fragment-arx-selection.test.tstests/link-creator.test.ts
Review fixes for #106: - Async link creation encoded the envelope twice (once per share surface), doubling the context-mixer cost. Candidates now carry both budgets and encodeEnvelopeSurfacesAsync selects both surfaces from one pool. - A forward-version priors asset failed every async encode; it now degrades to the shared prior on encode, and still refuses m/c/j decodes retryably. - The priors fetch gets a 10s timeout, and the asset version must be a non-negative integer to install. - CI installs WebKit for the new Playwright project and runs Node 24, matching the runtime the specs need. - arx4 is registered in the linking skills, AGENTS.md, and the architecture doc alongside the other codecs. - The priors build script now checks dictionary-prefix drift for real instead of comparing a value to itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The prior id list named n without saying what it means; s and n are now both defined next to the curated ids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The generated URL went through the URL serializer, which percent-encodes non-ASCII fragments, so Copy link delivered a 3x longer string than the visible form the fragment budget counts. The URL is now built by concatenating the fragment onto the serialized base, keeping the paste surface at its counted length; markdown links keep their ASCII wire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The version guard only rejected newer assets, so an older valid-shaped priors asset could encode curated m/c/j links that a current decoder cannot open. Curated priors now require the exact expected version on both paths: encode degrades to the shared prior on any skew, and decoding a curated fragment against a skewed or missing asset throws the retryable unavailable error. The expected version and the curated id list are single-sourced in arx4-codec.ts. Regression test replays the version-0 repro and was verified red before the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build-arx4-priors.mjs`:
- Line 45: Update the frozen source URL construction near FROZEN_SOURCE to
convert the CLI filesystem path with Node’s pathToFileURL utility instead of
manually creating a file URL. Preserve the existing frozenSourcePath input and
downstream URL usage while ensuring Windows absolute paths produce valid file
URLs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b50e050-cb1e-4f5f-aa0a-481596df1f7b
📒 Files selected for processing (17)
.github/workflows/ci.yml.github/workflows/test.ymlAGENTS.mddocs/architecture.mddocs/payload-format.mdpackage.jsonscripts/build-arx4-priors.mjsskills/agent-render-linking/SKILL.mdskills/selfhosted-agent-render/SKILL.mdsrc/lib/payload/arx4-codec.tssrc/lib/payload/fragment-arx.tssrc/lib/payload/fragment.tssrc/lib/payload/link-creator.tstests/arx4-priors-version-guard.test.tstests/arx4-priors.test.tstests/link-creator-encode-once.test.tstests/link-creator.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/architecture.md
- docs/payload-format.md
new URL(path, file://cwd) breaks on Windows drive letters (parsed as a URL scheme) and on paths containing # or spaces. readFileSync takes a plain resolved path, so no URL is involved at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Findings from two independent adversarial reviews, each reproduced red before its fix: - arx4 refuses to code on a non-pinned dictionary or overlay version: encode skips the arx4 candidate and decode throws a retryable skew error, so a fallback-dictionary session can no longer mint e links that healthy viewers cannot open. - The priors asset must reassemble to the pinned sha256 priors at install, which also bounds its size; a wrong-content version-1 asset is rejected and stays refetchable instead of caching as authoritative. - Curated-prior fetch routing decodes percent-escaped prior ids before deciding, so %6D fragments fetch the asset instead of wedging. - Markdown links always take the smaller transport candidate; the oversize fallback to the unicode fragment is gone. - An off-version priors fetch never sticky-installs mid-deploy, and arx4 asset failures surface as retryable asset-unavailable instead of invalid-json. No silent mis-decode path existed before or after; every skew fails closed. That property is now pinned by regression tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/payload/link-creator.ts (1)
209-256: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon’t select markdown fragments that exceed the app’s fragment budget.
selectMarkdownFragmentprefers the ASCII transport candidate purely by encoded length and skips theMAX_FRAGMENT_LENGTHcheck, while hash-page navigation decodes withskipFragmentBudget: undefined. Non-test navigation only passes{ skipFragmentBudget: true }for injected payloads, so the shared markdown link can be accepted by the link creator and returned to the user even though decoding it in-app fails before parsing withtoo-large. Fall back/reject when the markdown fragment is over budget, or surface a clear warning that this link may not load.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/payload/link-creator.ts` around lines 209 - 256, Update selectMarkdownFragment and its use in assembleGeneratedLink so the chosen markdown fragment never exceeds MAX_FRAGMENT_LENGTH. Validate the transport candidate’s visible fragment length before selecting it; if it is over budget, retain the primary fragment or reject the link consistently, ensuring generated markdown links remain loadable by normal hash-page navigation.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/payload/link-creator.ts`:
- Around line 209-256: Update selectMarkdownFragment and its use in
assembleGeneratedLink so the chosen markdown fragment never exceeds
MAX_FRAGMENT_LENGTH. Validate the transport candidate’s visible fragment length
before selecting it; if it is over budget, retain the primary fragment or reject
the link consistently, ensuring generated markdown links remain loadable by
normal hash-page navigation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 185ee637-584a-423d-8385-1d4fbb4c8efa
📒 Files selected for processing (15)
scripts/build-arx4-priors.mjssrc/lib/payload/arx4-codec.tssrc/lib/payload/fragment-arx.tssrc/lib/payload/fragment.tssrc/lib/payload/link-creator.tssrc/lib/payload/schema.tssrc/lib/sha256.tstests/arx4-curated-prior-fetch.test.tstests/arx4-dictionary-pin-guard.test.tstests/arx4-priors-fetch-skew.test.tstests/arx4-priors-version-guard.test.tstests/arx4-priors.test.tstests/e2e/arx4-determinism.spec.tstests/link-creator.test.tstests/sha256.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/build-arx4-priors.mjs
A right-version asset that failed the pinned-digest check returned -1 straight out of the URL loop, so a corrupt .br response blocked the intact plain .json fallback. The loop now moves on unless the install actually succeeded. Regression test verified red before the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
What this ships
The arx4 codec (tag
e). Same tuple, substitution, and dictionary stages as arx3, with brotli replaced by a deterministic integer context mixer that includes a column-position context for tables. Fragments code 10.3% smaller than arx3 on the sample fixtures (all seven win; CSV improves the most). Auto-selection puts arx4 first and falls back to arx3 wherever it loses, so links only ever get smaller.Curated priors as a lazy asset.
public/arx4-priors.json(13 KB brotli) primes the mixer per artifact kind. It is fetched only when a fragment actually names a curated prior; encode degrades gracefully to the shared prior when the asset is unavailable, and decode fails retryably rather than mis-decoding. Asset integrity is pinned by sha256 intests/arx4-priors.test.ts;scripts/build-arx4-priors.mjsregenerates it from the maintainer-held source and hard-fails on any mismatch.Markdown links no longer balloon. URL serializers percent-encode non-ASCII fragments, so a 76-char arx3 baseBMP fragment became a 704-char markdown link. Markdown links now carry a transport-budgeted ASCII encoding of the same payload (235 chars for that case); the paste URL keeps the dense form, and both decode identically.
Correctness
Compression is only useful if decode is bit-exact everywhere, so the riskiest part of this PR is determinism:
tests/fixtures/arx4-vectors.ts), so all engines are held to one copy of the strings.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
#e<payload>fragment tag.Documentation
Testing