Repository navigation
feat(core): implement Phase 3 decoder, reconstruction, timeline, and domain core - #4
Merged
Merged
Conversation
…gic+purity hooks Phase 3 T0. Creates the four pure-core test dirs with placeholder smoke tests so bun test:logic never errors on an empty dir; fixes the test:logic glob (drop non-existent ./lib/parser, add ./lib/decoder and ./lib/domain); re-enables the prek bun-logic-tests pre-push hook; adds scripts/check-pure-core.sh (greps #imports/browser./wxt under the four pure dirs) wired as a committed prek local hook so the purity invariant survives into Phase 4. Adds @types/bun + tsconfig types entry so the bun:test logic files type-check under tsc --noEmit, and excludes the bun-owned pure-core subdirs from Vitest (they import bun:test, which Vitest cannot resolve). Refs: .omc/plans/phase-3-core-plan.md T0 Signed-off-by: edbpede <144238562+edbpede@users.noreply.github.com>
…uctors Phase 3 T1. Adds lib/domain/ids.ts (branded DocId/RevisionId/SessionId/ UserId with validating as* constructors that throw on malformed input, plus unsafeAs* trusted-boundary blind casts) and lib/domain/model.ts (typed Document, RevisionRange, RawPayload, DecodedRevision, DocumentState, TimelineEvent union, PlaybackSession, CacheRecord, and privacy-safe DiagnosticReport — length-only/op-code tokens, no raw text). Introduces lib/decoder/types.ts (the type-only Operation grammar union, MIT attribution alongside AGPL) ahead of the T3 decode runtime, since both the domain model and the reconstruction engine depend on those shapes; the open-world decode funnel lands in T3. Refs: .omc/plans/phase-3-core-plan.md T1 Signed-off-by: edbpede <144238562+edbpede@users.noreply.github.com>
…ting schema-detect
Phase 3 T2. Adds lib/protocol/* as the single home for Google Docs
transport assumptions (§19): framing.ts (stripGuard fail-safe-detects the
)]}' guard, parseFramed -> JSON; lives ONLY here, R1), endpoints.ts
(revisions/load URL builder + /u/{N}/ multi-account matcher, A.5),
discovery.ts (typed range-discovery seam, UNCONFIRMED strategy, no live
calls), schema-detect.ts (detectSchema gates the hand-off: unknown shapes
never reach the decoder, §9.4), and types.ts (SchemaVersion + UNCONFIRMED
transport sentinels, each PROVISIONAL — pending §24).
Adds docs/protocol-capture.md with a STATUS: BLOCKED banner and all 12
§24 transport questions marked UNANSWERED/BLOCKED, plus the four
stop-conditions — escalated to the maintainer (un-performable by an agent).
Extends the test:logic glob to include ./lib/protocol so the bun framing/
schema-detect/endpoint tests actually execute (otherwise they would be
silently uncovered — pre-mortem #1).
Refs: .omc/plans/phase-3-core-plan.md T2
Signed-off-by: edbpede <144238562+edbpede@users.noreply.github.com>
…privacy-safe UnknownOp Phase 3 T3. Adds lib/decoder/decode.ts: decodeOperations(parsed: unknown) consumes already-parsed JSON from the protocol layer (never strips/parses, never imports lib/protocol — R1). The funnel switches over the raw wire ty:string; known literals (is/ds/mlti/iss/dss/msfd/usfd/opaque) build their typed variant, mlti recurses depth-first, and any unrecognized ty OR a known op with malformed fields degrades to UnknownOp with default -> no never (R2). UnknownOp carries opCode + byteLength only — never verbatim text (R5, §13.7); the test asserts a planted secret never survives decode. All index reads are noUncheckedIndexedAccess-guarded. Refs: .omc/plans/phase-3-core-plan.md T3 Signed-off-by: edbpede <144238562+edbpede@users.noreply.github.com>
…e and snapshotting Phase 3 T4. Adds the flat character-array model (model.ts): each element carries insertRevision, a tombstone deleteRevision (null=live), and a required suggestionState; the EndOfBody sentinel is a single reserved element guarded by isEndOfBody() (R3, R6, R12). apply.ts is the closed-world exhaustiveness gate — a switch over the typed Operation union with a never default (deleting a variant + its arm is a tsc error, R2); is splices at the ibi-th live position, ds tombstones the live si..ei range (never physically removed), msfd/dss mark-for-deletion WITHOUT setting deleteRevision, usfd resets, mlti recurses depth-first, opaque occupies a slot, unknown never mutates text. Wire indices address the live (deletion-collapsed) document per real A.2 semantics while tombstones are retained physically for time-travel. text.ts is a single O(N) filter (currentText / stateAt); snapshot.ts caches a model every N=100 revisions for scrub round-trips. Tests use hand-derived expected text. Refs: .omc/plans/phase-3-core-plan.md T4 Signed-off-by: edbpede <144238562+edbpede@users.noreply.github.com>
…venance Phase 3 T5. Adds lib/timeline/derive.ts: deriveTimeline(revisions) groups revisions into sessions (by session_id, splitting on a changed id or a temporal gap beyond the idle threshold), detects large insertions/ deletions (signed charDelta over a threshold), and detects pauses (inter-revision gaps). Every inferred grouping carries confidence (0..1) + a provenance string (PRD §9.5). Pure and deterministic; no clocks/random. Refs: .omc/plans/phase-3-core-plan.md T5 Signed-off-by: edbpede <144238562+edbpede@users.noreply.github.com>
…guard Phase 3 T6. Adds the single canonical lib/fixtures/ home (R12): corpus.ts holds hand-authored fixtures whose expectedFinalText is computed BY HAND from A.2 prose (never snapshotted apply output — R4), covering single insert/delete, nested mlti, suggestion lifecycle (iss/msfd/usfd), dss, opaque positioning, unknown-op isolation, and a multi-revision corpus; perf.ts builds a ~10k-revision corpus. The reconstruction fixtures.test.ts asserts the three tiers: [x:hand-derived] end text equals hand-computed text, [x:internal] snapshot-scrub round-trip equals linear replay, and the O(N) stateAt guard (no per-revision mutation). [BLOCKED:live] real-document equality stays escalated. README.md records the tier scheme + the no- gdocrevisions-sample-data rule. Refs: .omc/plans/phase-3-core-plan.md T6 Signed-off-by: edbpede <144238562+edbpede@users.noreply.github.com>
…escalation Phase 3 T7. Records the agent-completable [x] vs human-only [BLOCKED] vs [DEFERRED:Phase 6] acceptance mapping, the verification-command snapshot (all gates green, seam/purity/.raw/never-gate proven), the three flagged plan deviations, and the maintainer escalation note for the §24 live capture that gates Phase 4. Refs: .omc/plans/phase-3-core-plan.md T7 Signed-off-by: edbpede <144238562+edbpede@users.noreply.github.com>
…export internal clone Phase 3 post-review deslop pass (changed-file scope). Replaces the raw `el.kind !== "eob"` check in tombstoneRange with the plan-mandated isEndOfBody() predicate (making the previously comment-only export load-bearing), and un-exports cloneElement (only cloneModel uses it). Behavior-preserving: all 78 bun tests, tsc, biome, and check-pure-core stay green. Refs: .omc/plans/phase-3-core-plan.md T7 Signed-off-by: edbpede <144238562+edbpede@users.noreply.github.com>
|
Warning Review limit reached
More reviews will be available in 5 minutes and 35 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 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (31)
📝 WalkthroughWalkthroughPhase 3 implements the complete "pure core" for DocRewind: a decoder that transforms wire operations into typed domain operations, a reconstruction pipeline that applies those operations to a document model via tombstone-based semantics, timeline derivation that infers high-level editing events, comprehensive test fixtures and validation, and infrastructure guards enforcing pure-core constraints. ChangesPhase 3 Pure Core Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 12
🤖 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 `@lib/decoder/decode.test.ts`:
- Around line 118-130: Add two regression tests in decode.test.ts mirroring the
existing malformed-op tests: use onlyOp(entry(...)) to create the ops and assert
they downgrade to unknown. First, create an op with ty: "rg" where si > ei
(e.g., si: 10, ei: 1) and expect op.ty === "unknown" and op.opCode === "rg";
second, create an op with ty: "mlti" where mlti.mts is not an array (e.g., mlti:
{ mts: "not-array" }) and expect op.ty === "unknown" and op.opCode === "mlti".
Ensure the tests follow the same pattern as the existing cases (use the
sentinel-missing guard style) so they exercise the downgrade-to-unknown behavior
in onlyOp/entry.
In `@lib/decoder/decode.ts`:
- Around line 91-98: The decoder currently accepts delete-family ranges where
the start index exceeds the end (si > ei); update the decode logic in the "ds"
case to reject reversed inclusive ranges by checking if si === undefined || ei
=== undefined || si > ei and returning unknownOp(raw, ty, revisionId) in that
situation, and apply the same validation to the other delete-family cases
("dss", "msfd", "usfd") that use asPositiveInt(field(...)) so all reversed
ranges are downgraded to unknownOp; reference the case "ds"/"dss"/"msfd"/"usfd",
asPositiveInt, field, and unknownOp when making the change.
- Around line 99-106: The "mlti" branch currently coerces a non-array mts into
[] and returns a valid mlti op, which hides malformed input; instead, in the
case handler for "mlti" (the switch case shown), verify that mts is an array and
if not return an UnknownOp (downgrade) preserving the original raw payload; if
mts is an array, map each sub through decodeOperation(sub, revisionId) as
before. Ensure you reference the same symbols: case "mlti", field(raw, "mts"),
decodeOperation(..., revisionId), and return an UnknownOp value (using the
project's UnknownOp shape) when mts is not an array.
- Around line 52-57: The byteLengthOf function currently uses serialized.length
(UTF-16 code units) which miscounts non-ASCII bytes; update byteLengthOf to
compute the actual UTF-8 byte length of JSON.stringify(raw) (e.g., use
TextEncoder.encode(serialized).length in browser/modern runtimes or
Buffer.byteLength(serialized, "utf8") in Node) and return that value so
UnknownOp.byteLength reports correct UTF-8 sizes; keep the try/catch and
fallback to 0 on error and only replace the length calculation inside
byteLengthOf.
In `@lib/fixtures/perf.ts`:
- Around line 21-30: The buildLinearInsertCorpus function should validate its n
parameter before the for loop: ensure n is a finite, non-negative integer (use
Number.isFinite(n) && Number.isInteger(n) && n >= 0) and either throw a clear
RangeError or clamp/normalize n to a safe value if invalid, so the loop that
uses i < n cannot run forever (protect the changelog generation and
expectedFinalText construction from Infinity/NaN).
In `@lib/protocol/discovery.ts`:
- Around line 11-37: Change the RevisionRangeDiscovery seam to accept the
branded DocId instead of raw string: import the DocId (branded) type from
"../domain/ids" alongside RevisionId and update the interface signature
discoverUpperBound(docId: DocId): Promise<RevisionId>; also update any
references/implementations of RevisionRangeDiscovery and callers of
discoverUpperBound to pass/validate DocId values so the domain/protocol boundary
preserves the identifier contract.
In `@lib/protocol/endpoints.ts`:
- Around line 27-49: detectUserIndex currently scans the entire URL string and
USER_INDEX_PATTERN can match query/fragment; also buildRevisionsLoadUrl
interpolates params.userIndex without validating it. Fix detectUserIndex to
parse the URL via the URL constructor and run a stricter regex against
url.pathname only (e.g. anchor to start or segment boundaries like
/^\/u\/(\d+)(?:\/|$)/) so it cannot match query/fragment; in
buildRevisionsLoadUrl validate params.userIndex before using it (ensure
Number.isInteger(params.userIndex) && params.userIndex >= 0) and only include
the /u/{N} segment when it passes validation.
In `@lib/protocol/framing.ts`:
- Around line 8-10: The file defines a duplicate GUARD_PREFIX constant; replace
the local GUARD_PREFIX with the canonical value by importing DEFAULT_TRANSPORT
from lib/protocol/types.ts and using DEFAULT_TRANSPORT.guardPrefix wherever
GUARD_PREFIX was referenced (remove the local GUARD_PREFIX declaration),
ensuring framing logic uses the single source of truth
DEFAULT_TRANSPORT.guardPrefix.
In `@lib/reconstruction/apply.ts`:
- Around line 82-91: The loop over chars continues scanning after the inclusive
end index (ei) is reached; modify the for-loop in apply.ts (the block that
iterates "for (const el of chars)" using count, si, ei, isEndOfBody,
el.deleteRevision, and revisionId) to short-circuit by breaking out once count >
ei (or immediately after applying the final deletion when count === ei) to avoid
unnecessary work; apply the same early-break fix to the corresponding second
range-walk block around the 101-110 region so both walkers stop scanning after
the target range is fully processed.
In `@lib/timeline/derive.test.ts`:
- Around line 11-103: Add a new test in derive.test.ts that calls
deriveTimeline(decode(...)) with a mixed sequence of input ops that will produce
a session event, a large-insertion/large-deletion event, and a pause (use varied
revision_id and time values to enforce ordering across those outputs), then
assert the returned events array preserves revision/time order (e.g. map events
to their kind or check indexOf for "session",
"large-insertion"/"large-deletion", and "pause") instead of grouping by event
type; reference deriveTimeline and decode to build the input and use the
event.kind strings ("session", "large-insertion"/"large-deletion", "pause") to
locate and assert the correct ordering.
In `@lib/timeline/derive.ts`:
- Around line 193-197: The returned timeline events are being concatenated by
type (deriveSessions, deriveLargeEdits, derivePauses) which breaks chronological
order; change the return in derive.ts to merge the three arrays and then sort
the combined array by the event timestamp field (e.g., event.time or
event.timestamp) so events flow in revision/time progression; if timestamps can
be equal, add a secondary stable key such as revision index or event.type to
preserve deterministic ordering; update the return of derive.ts to return the
sorted merged array instead of the raw concatenation.
In `@prek.toml`:
- Around line 53-56: The purity-guard scope is inconsistent: vitest.config.ts
treats "pure-core tiers" as including lib/protocol/** and lib/fixtures/** but
scripts/check-pure-core.sh only checks PURE_DIRS =
lib/{decoder,reconstruction,timeline,domain}; update the invariant so they match
by either (A) extending PURE_DIRS inside scripts/check-pure-core.sh to also
include lib/protocol and lib/fixtures (and update the script comment) or (B)
change the prek.toml entry for id "check-pure-core" (and its description) to
state the guard only covers the four existing dirs; reference the script name
(scripts/check-pure-core.sh), the PURE_DIRS variable, and the prek.toml
"check-pure-core" entry when making the change so vitest.config.ts and the guard
remain consistent.
🪄 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: 483bf5c0-a9af-4da6-9b8f-3c93532beae8
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
docs/phase-3-acceptance.mddocs/protocol-capture.mdlib/decoder/decode.test.tslib/decoder/decode.tslib/decoder/types.tslib/domain/ids.test.tslib/domain/ids.tslib/domain/model.tslib/fixtures/README.mdlib/fixtures/corpus.tslib/fixtures/perf.tslib/protocol/discovery.tslib/protocol/endpoints.tslib/protocol/framing.test.tslib/protocol/framing.tslib/protocol/schema-detect.tslib/protocol/types.tslib/reconstruction/apply.test.tslib/reconstruction/apply.tslib/reconstruction/fixtures.test.tslib/reconstruction/model.tslib/reconstruction/snapshot.test.tslib/reconstruction/snapshot.tslib/reconstruction/text.tslib/timeline/derive.test.tslib/timeline/derive.tspackage.jsonprek.tomlscripts/check-pure-core.shtsconfig.jsonvitest.config.ts
- decode.ts: downgrade reversed inclusive ranges (si>ei) in ds/dss/msfd/usfd to UnknownOp - decode.ts: downgrade non-array mlti.mts to UnknownOp instead of coercing to [] - decode.test.ts: add reversed-range (ds) and non-array-mts regression tests - perf.ts: guard buildLinearInsertCorpus(n) against Infinity/NaN/negative via TypeError - discovery.ts: type discoverUpperBound docId as branded DocId, not raw string - endpoints.ts: validate userIndex and scope detectUserIndex to URL pathname - framing.ts: derive GUARD_PREFIX from DEFAULT_TRANSPORT (single source of truth) - apply.ts: short-circuit both range walkers once count exceeds ei - derive.ts: return timeline events in chronological (revision) order - derive.test.ts: add cross-kind ordering regression test - check-pure-core.sh/prek.toml: extend purity guard to protocol + fixtures tiers
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements Phase 3 (core architecture) of DocRewind per
.omc/plans/phase-3-core-plan.md: the pure, browser-API-free decoder / reconstruction / timeline / domain core, plus an isolated, fail-safe protocol skeleton. No live network capture is performed (it is un-performable by an automated agent and is escalated to the maintainer — see Blocked below).All work follows
.augment/rules/bun-solid-pro.md(strict TypeScript, noany,verbatimModuleSyntax,exactOptionalPropertyTypes,noUncheckedIndexedAccess;bun testonly for DOM-free logic) and the PRD Appendix A.2 operation grammar.What this delivers
lib/domain/) — brandedDocId/RevisionId/SessionId/UserIdwith validatingas*constructors (throw on malformed input) and justifiedunsafeAs*trusted-boundary casts; the full typed model (Document,RevisionRange,RawPayload,DecodedRevision,TimelineEvent,PlaybackSession,CacheRecord, and a privacy-safeDiagnosticReport).lib/decoder/) —decodeOperations(parsed: unknown)consumes already-parsed JSON via an open-world wire-tyfunnel; known literals build their typed variant,mltirecurses depth-first, and any unrecognizedty(or a known op with malformed fields) degrades to a privacy-safeUnknownOp(op-code + byte length only, never verbatim text). The funnel has noneverby design.lib/protocol/) — the single home for Google Docs transport assumptions:stripGuard/parseFramed(fail-safe)]}'strip), therevisions/loadURL builder +/u/{N}/multi-account matcher, a typed range-discovery seam, anddetectSchemawhich gates the hand-off (unknown shapes never reach the decoder). All transport constants areUNCONFIRMED/PROVISIONALpending the live capture.lib/reconstruction/) — a flat tombstone character-array model (deletes setdeleteRevision, never physically removed), withapply.tsas the closed-world exhaustiveness gate (aneverdefault makes a missing arm a compile error). Wire indices address the live (deletion-collapsed) document per real A.2 semantics and are mapped to physical indices while tombstones are retained for time-travel.stateAtis a single O(N) filter; snapshotting (N=100 cadence) supports efficient scrubbing.lib/timeline/) —deriveTimelinegroups revisions into sessions (bysession_idand temporal gaps), detects large insertions/deletions and pauses, and tags every inferred grouping with confidence + provenance.lib/fixtures/) — hand-authored synthetic fixtures whoseexpectedFinalTextis computed by hand from A.2 prose (never by snapshottingapply.tsoutput), plus a ~10k-revision perf-shaped corpus that guards the O(N)stateAtshape.scripts/check-pure-core.sh(committed prek hook) fails the build if#imports/browser./wxtleak into the four pure dirs; thetest:logicglob is fixed and the prekbun-logic-testshook is re-enabled.Design decisions
ty: stringwithdefault -> UnknownOp(nonever); the reconstructionapply.tsswitches over the typedOperationunion with aneverdefault (the real gate). Deleting a variant and its arm is atscerror — verified.UnknownOpand all diagnostic shapes carry only op-codes and length tokens; a test plants a secret payload and asserts it never survives decode.Verification
bun run compile(tsc --noEmit) — cleanbun run check(Biome) — clean; noany, no@ts-ignorebun run test:logic— 78 pass / 0 fail (7 files)bun run test:run(Vitest) — pass (existing Phase 2 smoke; no Phase 3 Vitest specs)bash scripts/check-pure-core.sh— exit 0lib/protocolimport orstripGuardunderlib/decoder; no.rawunderlib/decoder/lib/domainprek run --all-files(commit and pre-push stages) — greenAn independent architect review verified all acceptance criteria, hand-derived multiple fixtures, and adversarially traced the live-position model; no correctness bugs were found.
Blocked / escalated (out of scope for this PR)
The PRD §24 live network capture (authenticated Chromium + Firefox against three documents, including a multi-account
/u/1/session) is un-performable by an automated agent and is escalated to the maintainer. All 12 transport questions indocs/protocol-capture.mdare marked BLOCKED. Phase 4 (network retrieval) must not start until that capture is recorded, and must halt if it reveals protobuf, abatchexecutewrapper, a new mandatory read token, or endpoint restrictions. The honest acceptance mapping is indocs/phase-3-acceptance.md.Deviations from the plan (documented in docs/phase-3-acceptance.md)
lib/decoder/types.ts(theOperationunion) landed with the domain commit because both the domain model and the reconstruction engine depend on it; the decode runtime still lands separately.test:logicglob includes./lib/protocolso the protocol bun tests actually execute (the plan's own test-tiering table classifies them as bun pure-logic; omitting them would be a silent-coverage gap).@types/bun(so thebun:testfiles type-check undertsc) and excluded the bun-owned pure-core subdirs from Vitest (they importbun:test, which Vitest cannot resolve).Notes
Signed-off-by.AGPL-3.0-or-laterheaders are present;lib/decoder/decode.tsandlib/decoder/types.tsadditionally carry the MIT attribution for the portedgdocrevisionsgrammar.Summary by CodeRabbit
Release Notes
New Features
Tests
Documentation
Chores