Document graph-backed rope runtime gate - #205
Conversation
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 Run ID: 📒 Files selected for processing (3)
WalkthroughAdds a new design gate document (docs/design/0149-graph-backed-rope-runtime-discovery.md) describing the architectural drift between jedit's current full-snapshot text runtime and the intended graph-backed rope runtime, specifying coordinate systems, typed fact shapes, retention/materialization rules, and a phased cutover plan. ChangesDesign documentation cohort
Estimated code review effort: 2 (Simple) | ~12 minutes Since this is a documentation-only addition with no code paths, control flow, or multi-component interactions to visualize, no sequence diagram is warranted. Related Issues: None referenced. Related PRs: None referenced. Suggested labels: documentation, design-doc Suggested reviewers: flyingrobots Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/design/0149-graph-backed-rope-runtime-discovery.md`:
- Around line 165-168: Clarify the LineColumn contract by explicitly stating
whether line is 0-based or 1-based, and document how newline normalization is
performed before deriving coordinates. Update the LineColumn interface
documentation in the graph-backed rope runtime discovery spec so adapters can
rely on the same basis when producing line and columnUtf16 values.
- Around line 286-300: The current contract check in the rope runtime discovery
example only compares `debugRopeShape` at one level (`untouchedLeftSubtreeId`
and `untouchedRightSubtreeId`), which is too shallow. Update the
`runtime.debugRopeShape` / `runtime.replaceRangeAsTick` contract example to use
a recursive structural witness that verifies all unaffected spans and subtree
identities, not just the immediate left/right subtrees. Keep the comparison
centered on the `before` and `after` shapes, but assert preservation across the
full untouched structure so the contract remains valid even when deeper
rebalance occurs.
- Around line 159-243: The fact model is currently only described with erased
TypeScript brands and string discriminants, so witnesses cannot rely on a real
runtime shape. In the graph-backed rope runtime section, add a canonical
constructor/validator path for the fact objects referenced by
BufferWorldlineFact, RopeHeadFact, RopeBranchFact, RopeLeafFact, and
TextBlobFact, and specify the runtime tags or payload fields they carry. Keep
the branded aliases as compile-time helpers, but make the design explicitly
define how these facts are created and validated at runtime so inspectors can
target stable object shapes rather than ad hoc strings.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8d735d37-6bb2-4b16-bb66-8c1488c13a0f
📒 Files selected for processing (1)
docs/design/0149-graph-backed-rope-runtime-discovery.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: test / docs-release
🧰 Additional context used
📓 Path-based instructions (2)
docs/design/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Use the template from
docs/design/TEMPLATE.mdfor all full cycle designs
Files:
docs/design/0149-graph-backed-rope-runtime-discovery.md
**
⚙️ CodeRabbit configuration file
**: # jedit Agent ContractGit Safety
- Never amend commits. Make a new commit.
- Never rebase unless the user explicitly approves it after a concrete explanation.
- Never force git operations.
- Draft pull requests are allowed only for design-cycle kickoff or active cycle
work. Convert them to ready before merge. If draft PRs are unavailable, use a
normal PR with thework-in-progresslabel.Quality Doctrine
These are hard repo rules, not suggestions:
- No
any.- No
unknown.- Hexagonal architecture, strict.
- Encoding and decoding happen only at the boundary, inside a port/adapter.
- One file equals one runtime truth.
- Runtime truth beats compile-time theater.
- No ad hoc string comparison in core logic. Prefer
instanceof, numeric tags, symbols, or explicit runtime objects.- No magic strings.
- No magic numbers.
- SOLID.
- DRY.
- DI.
- Red to green.
- Tests are spec.
- Agent-first.
- No file over 500 lines of code.
Structure
ARCHITECTURE.mdis the canonical repo architecture doctrine.docs/BEARING.mdrecords the current execution gravity.docs/method/process.mdis the canonical cycle workflow.docs/method/roadmap-planning.mddefines release-gate, roadmap, slice, and
proof policy for multi-cycle planning.docs/design/TEMPLATE.mdis the required template for full cycle designs.docs/design/0034-design-cycle-template-and-lifecycle.mdrecords the policy
decision that made executable design docs official.docs/design/0024-jedit-powered-by-echo-release-gate.mdis the completed
baseline plan for the first thirty slices proving jedit has an installed
package/evidence path while Echo remains generic.docs/design/0025-echo-application-hosting-pattern.mdis the completed
post-release-pressure ten-slice plan for proving the reusable Echo
application-hosting pattern before applying it to Graft, Think, or other apps.docs/design/0026-echo-hosting-hardening-first-twenty.mdis th...
Files:
docs/design/0149-graph-backed-rope-runtime-discovery.md
🪛 LanguageTool
docs/design/0149-graph-backed-rope-runtime-discovery.md
[uncategorized] ~14-~14: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...The current hot text runtime still uses full text snapshots: - [`src/domain/text-edit-co...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~371-~371: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ity; - repeated edits do not retain one full text snapshot per edit; - untouched subtree ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~533-~533: Ensure spelling is correct
Context: ...int to an actual rope. - Do not make UI truthier than storage truth. - Do not mix cache ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~535-~535: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...invalidation with authority mutation. - Do not make compaction destroy explainabil...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~535-~535: ‘by accident’ might be wordy. Consider a shorter alternative.
Context: ... make compaction destroy explainability by accident. ### 16. Treat UI Work As Dependent On...
(EN_WORDINESS_PREMIUM_BY_ACCIDENT)
Self-Code Review@codex please confirm these findings. I reviewed Findings
Severity breakdown
Verification performed
Review judgmentChanges requested. The discovery document is directionally correct and worth keeping, but the design gate should not merge while the coordinate contract, runtime fact validation path, and untouched-structure witness remain underspecified. |
Review Fix SummaryResolved the self-review / CodeRabbit findings in
Local validation after the fix:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 004733e741
ℹ️ 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".
|
Resolved the discovered review issues in follow-up commits
Validation run locally: git diff --check
npx markdownlint-cli2 docs/BEARING.md docs/design/0149-graph-backed-rope-runtime-discovery.md
node --test --test-concurrency=1 spec/design-cycle-policy.spec.mjs
npm run --silent qualityAll previously unresolved review threads are now resolved. Latest PR state: checks green, CodeRabbit approved, merge state clean. |
Code Lawyer Self-Audit Findings@codex please confirm these findings before merge.
I am resolving these now. The first finding will get a red/green docs policy witness and commit; the second is PR metadata and will be resolved by editing the PR body because there is no repository artifact to commit. |
Code Lawyer Activity Summary
Validation: git status --porcelain
git fetch origin
git diff origin/main...HEAD
git diff --check
npx markdownlint-cli2 docs/design/0149-graph-backed-rope-runtime-discovery.md spec/design-cycle-policy.spec.mjs
node --test --test-concurrency=1 spec/design-cycle-policy.spec.mjs
npm run checkLocal result: Remote result on Merge gate remains locked by CodeRabbit prepaid credits exhaustion on the latest head and by insufficient current approvals. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a302437eb5
ℹ️ 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".
Activity Summary
Validation completed locally:
All review threads known at the start of this Code Lawyer pass are resolved. No new self-discovered issues were found during the final diff audit. |
Summary
Related
Verification
Notes
This PR is intentionally documentation/design only. It does not implement the graph-backed runtime and does not touch PR #204.