fix!: default checkpointPolicy to { every: 64 } - #842
Conversation
Auto-checkpointing is what bounds replay depth, but it was opt-in: RuntimeHost stored `checkpointPolicy || null` and `_tryAutoCheckpoint` returns immediately on a null policy. A caller that never supplied one therefore replayed its entire patch history since the last explicit checkpoint on every materialize, with no upper bound, and reads paid the cost. Measured on a real store at 262 unreplayed patches: one read spawned 5,267 Git subprocesses, and the backlog grew by two commits per write forever. Nothing surfaced this — the degradation is silent and monotonic. An omitted policy now takes DEFAULT_CHECKPOINT_POLICY. `checkpointPolicy: null` remains the explicit opt-out, so no-compaction stays reachable by asking for it rather than by forgetting. `every` is compared against the replay depth reported by materialize, not writes performed by the current process, so short-lived callers still compact once the backlog crosses the threshold. Two existing cases asserted the old contract and are updated deliberately: WarpGraph.checkpointPolicy 'defaults _checkpointPolicy to null when not provided' and WarpOpenOptions 'freezes required runtime open options'. A new case pins the null-vs-omitted distinction so the opt-out cannot regress into the default. Full unit suite: 7299 passed, 0 failed, 2 skipped. BREAKING CHANGE: graphs opened without an explicit checkpointPolicy now write checkpoint commits once replay depth reaches 64 patches. State hashes are unaffected; a checkpoint is a snapshot, not a semantic change. Pass checkpointPolicy: null to restore the previous behaviour.
|
Warning Review limit reached
Next review available in: 23 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe runtime host now applies an immutable default checkpoint policy of ChangesCheckpoint policy behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@CHANGELOG.md`:
- Around line 24-26: Update the changelog wording around checkpoint creation to
state that replay depth “reaches or exceeds 64,” making the inclusive threshold
explicit instead of implying it must exceed 64.
In `@src/domain/warp/RuntimeHostBoot.ts`:
- Around line 215-217: Replace the structural checkpoint policy with a
validated, frozen runtime-backed CheckpointPolicy value object. In
src/domain/warp/RuntimeHostBoot.ts, update lines 215-217 to construct
DEFAULT_CHECKPOINT_POLICY via CheckpointPolicy, expose CheckpointPolicy in
construction options at lines 57-57 and normalized options at lines 116-116, and
update the raw-input validation at lines 226-246 to construct and return the
value object, using instanceof dispatch at the domain boundary.
In `@test/unit/domain/WarpGraph.checkpointPolicy.test.ts`:
- Around line 39-40: Add an explicit equality assertion for
DEFAULT_CHECKPOINT_POLICY.every in the checkpoint policy test, requiring the
contractual value 64 while retaining the existing positivity and policy
assertions.
- Around line 115-119: Update the opted-out test’s openRuntimeHostProduct call
to pass null directly as checkpointPolicy, removing the unnecessary any cast and
preserving the test’s verification of the nullable TypeScript contract.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d57c220f-8e8a-4b14-adcf-4148d6e67652
📒 Files selected for processing (4)
CHANGELOG.mdsrc/domain/warp/RuntimeHostBoot.tstest/unit/domain/WarpGraph.checkpointPolicy.test.tstest/unit/domain/warp/WarpOpenOptions.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: test-bun
- GitHub Check: test-deno
- GitHub Check: test-node (22)
- GitHub Check: type-firewall-generated-sdk
- GitHub Check: coverage-threshold
- GitHub Check: type-firewall-lint
- GitHub Check: v19 base/head performance
- GitHub Check: preflight
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Issue Reference / 0_require-issue-reference.txt: fix!: default checkpointPolicy to { every: 64 }
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mnode <<'NODE'�[0m
�[36;1mconst fs = require('node:fs');�[0m
�[36;1mconst https = require('node:https');�[0m
�[36;1m�[0m
�[36;1mconst event = JSON.parse(fs.readFileSync(process.env.GITHUB_EVENT_PATH, 'utf8'));�[0m
�[36;1mconst ***REDACTED_SECRET_ASSIGNMENT***
�[36;1mconst pr = event.pull_request;�[0m
�[36;1mconst repository = event.repository.full_name;�[0m
�[36;1mconst [owner, repo] = repository.split('/');�[0m
�[36;1mconst text = `${pr.title ?? ''}\n${pr.body ?? ''}`;�[0m
�[36;1m�[0m
�[36;1mconst escapeRegExp = (value) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');�[0m
�[36;1mconst numbers = new Set();�[0m
�[36;1m�[0m
�[36;1mconst addNumber = (value) => {�[0m
�[36;1m const number = Number(value);�[0m
�[36;1m if (Number.isSafeInteger(number) && number > 0) {�[0m
�[36;1m numbers.add(number);�[0m
�[36;1m }�[0m
�[36;1m};�[0m
�[36;1m�[0m
�[36;1mfor (const match of text.matchAll(/(^|[^\w/-])#([1-9]\d*)\b/g)) {�[0m
�[36;1m addNumber(match[2]);�[0m
�[36;1m}�[0m
�[36;1m�[0m
�[36;1mfor (const match of text.matchAll(/\bGH-([1-9]\d*)\b/gi)) {�[0m
�[36;1m addNumber(match[1]);�[0m
�[36;1m}�[0m
�[36;1m�[0m
�[36;1mconst sameRepo = escapeRegExp(repository);�[0m
�[36;1mfor (const match of text.matchAll(new RegExp(`\\b${sameRepo}#([1-9]\\d*)\\b`, 'gi'))) {�[0m
�[36;1m addNumber(match[1]);�[0m
�[36;1m}�[0m
�[36;1m�[0m
�[36;1mfor (const match of text.matchAll(�[0m
�[36;1m new RegExp(`https://github\\.com/${sameRepo}/issues/([1-9]\\d*)\\b`, 'gi'),�[0m
�[36;1m)) {�[0m
�[36;1m addNumber(match[1]);�[0m
�[36;1m}�[0m
�[36;1m�[0m
�[36;1mnumbers.delete(Number(pr.number));�[0m
�[36;1m�[0m
�[36;1mconst requestIssue = (number) =>�[0m
�[36;1m new Promise((resolve, reject) => {�[0m
�[36;1m const request = https.request(�[0m
�[36;1m {�[0m
�[36;1m hostname: 'api.github.com',�[0m
�[36;1m method: 'GET',�[0m
�[36;1m path: `/repos/...
GitHub Actions: PR Issue Reference / require-issue-reference: fix!: default checkpointPolicy to { every: 64 }
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mnode <<'NODE'�[0m
�[36;1mconst fs = require('node:fs');�[0m
�[36;1mconst https = require('node:https');�[0m
�[36;1m�[0m
�[36;1mconst event = JSON.parse(fs.readFileSync(process.env.GITHUB_EVENT_PATH, 'utf8'));�[0m
�[36;1mconst ***REDACTED_SECRET_ASSIGNMENT***
�[36;1mconst pr = event.pull_request;�[0m
�[36;1mconst repository = event.repository.full_name;�[0m
�[36;1mconst [owner, repo] = repository.split('/');�[0m
�[36;1mconst text = `${pr.title ?? ''}\n${pr.body ?? ''}`;�[0m
�[36;1m�[0m
�[36;1mconst escapeRegExp = (value) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');�[0m
�[36;1mconst numbers = new Set();�[0m
�[36;1m�[0m
�[36;1mconst addNumber = (value) => {�[0m
�[36;1m const number = Number(value);�[0m
�[36;1m if (Number.isSafeInteger(number) && number > 0) {�[0m
�[36;1m numbers.add(number);�[0m
�[36;1m }�[0m
�[36;1m};�[0m
�[36;1m�[0m
�[36;1mfor (const match of text.matchAll(/(^|[^\w/-])#([1-9]\d*)\b/g)) {�[0m
�[36;1m addNumber(match[2]);�[0m
�[36;1m}�[0m
�[36;1m�[0m
�[36;1mfor (const match of text.matchAll(/\bGH-([1-9]\d*)\b/gi)) {�[0m
�[36;1m addNumber(match[1]);�[0m
�[36;1m}�[0m
�[36;1m�[0m
�[36;1mconst sameRepo = escapeRegExp(repository);�[0m
�[36;1mfor (const match of text.matchAll(new RegExp(`\\b${sameRepo}#([1-9]\\d*)\\b`, 'gi'))) {�[0m
�[36;1m addNumber(match[1]);�[0m
�[36;1m}�[0m
�[36;1m�[0m
�[36;1mfor (const match of text.matchAll(�[0m
�[36;1m new RegExp(`https://github\\.com/${sameRepo}/issues/([1-9]\\d*)\\b`, 'gi'),�[0m
�[36;1m)) {�[0m
�[36;1m addNumber(match[1]);�[0m
�[36;1m}�[0m
�[36;1m�[0m
�[36;1mnumbers.delete(Number(pr.number));�[0m
�[36;1m�[0m
�[36;1mconst requestIssue = (number) =>�[0m
�[36;1m new Promise((resolve, reject) => {�[0m
�[36;1m const request = https.request(�[0m
�[36;1m {�[0m
�[36;1m hostname: 'api.github.com',�[0m
�[36;1m method: 'GET',�[0m
�[36;1m path: `/repos/...
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not use direct imports fromsrc/infrastructure/**insrc/domain/**orsrc/ports/**; depend on a port instead.
Do not use direct Node built-ins insrc/domain/**orsrc/ports/**; use a port instead.
Files:
test/unit/domain/WarpGraph.checkpointPolicy.test.tssrc/domain/warp/RuntimeHostBoot.tstest/unit/domain/warp/WarpOpenOptions.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,js,jsx}: Do not introduceany,as any,as unknown as,unknown(outside adapters),Record<string, unknown>(outside adapters),*Likeplaceholder types,JSON.parse/JSON.stringify(outside adapters),fetch(outside adapters),process.env(outside adapters),@ts-ignore, orz.any()in core code; use validated boundary models and ports instead.
Use constructor-injected ports for external capabilities; do not rely on ambient dependencies for I/O, clocks, persistence, or entropy.
Do not createutils.ts,helpers.ts,misc.ts, orcommon.ts; name files after the actual concept they model.
Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.
Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.
No enum usage; prefer runtime-backed domain forms and unions.
Do not use boolean trap parameters; prefer named option objects or separate methods.
Avoid magic strings or numbers when a named constant should exist.
Keep domain bytes asUint8Array;Bufferbelongs in infrastructure adapters.
Files:
src/domain/warp/RuntimeHostBoot.ts
src/domain/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/domain/**/*.{ts,tsx,js,jsx}: Insrc/domain/**, do not useDate.now(),new Date(),Date(),performance.now(),Math.random(),crypto.randomUUID(),crypto.getRandomValues(),setTimeout,setInterval, rawnew Error(...)/new TypeError(...), or direct imports from Node built-ins; time, entropy, and external capabilities must enter through ports or parameters, and domain errors should extendWarpError.
Construct domain objects only in core when doing so establishes validated runtime truth; do not build infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core.
Prefer discriminated unions and explicit result types instead of boolean-flag bags, and model expected failures as return values rather than exceptions.
src/domain/must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.
Domain code must not use the wall clock directly; time must enter through a port or parameter.
Files:
src/domain/warp/RuntimeHostBoot.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit domain concepts with validated constructors,
Object.freeze, andinstanceofdispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.
Files:
src/domain/warp/RuntimeHostBoot.ts
🧠 Learnings (1)
📚 Learning: 2026-03-08T19:50:17.519Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 65
File: CHANGELOG.md:88-88
Timestamp: 2026-03-08T19:50:17.519Z
Learning: Follow the Keep a Changelog convention for CHANGELOG.md. Allow duplicate subheadings across versions (e.g., '### Added', '### Fixed'). Configure markdownlint MD024 with {"siblings_only": true} to avoid cross-version false positives.
Applied to files:
CHANGELOG.md
Release Preflight
If this PR is from a |
Code Lawyer self-audit — newly discovered issuesFull
@codex second opinion requested: please challenge the severity, completeness, and proposed boundaries before merge. No fixes are included in this comment; the findings will be handled one at a time with RED → GREEN → VERIFY → COMMIT evidence. |
|
@codex Second-opinion requested on one additional self-audit finding discovered during the touched-file SSJS scorecard.
|
Release Preflight
If this PR is from a |
Release Preflight
If this PR is from a |
Release Preflight
If this PR is from a |
|
@codex Final-diff second opinion requested on two new self-audit findings introduced during the repair loop.
|
Release Preflight
If this PR is from a |
Release Preflight
If this PR is from a |
Release Preflight
If this PR is from a |
Release Preflight
If this PR is from a |
Activity Summary
Verification
@codex second-opinion request: please review the exact-head invariants, especially derivative |
1 participant
Summary
checkpointPolicyto a bounded cadence of 64 replayed patches.checkpointPolicy: nullas the explicit no-compaction opt-out.Omitting
checkpointPolicysilently disabled auto-checkpointing, so replay depth was unbounded:Measured on a real store at 262 unreplayed patches: one read spawned 5,267 Git subprocesses, and the backlog grew by two commits per write. The degradation was silent and monotonic.
checkpointPolicynullBreaking change
Graphs opened without an explicit policy now write checkpoint commits once replay depth reaches or exceeds 64. State hashes are unaffected because a checkpoint is a snapshot, not a semantic graph change. Pass
checkpointPolicy: nullto retain no-compaction behavior.Issue
Closes #843
Test plan
Current branch baseline:
npm run lintnpm run typecheck -- --pretty falseADR checks