Skip to content

Add document-format RFC draft#4264

Open
TrueDoctor wants to merge 8 commits into
masterfrom
document-format-rfc
Open

Add document-format RFC draft#4264
TrueDoctor wants to merge 8 commits into
masterfrom
document-format-rfc

Conversation

@TrueDoctor

Copy link
Copy Markdown
Member

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new RFC document detailing the .gdd document format for Graphite, which decouples the on-disk layout from the editor's in-memory runtime types using a flat node registry and operation-based CRDT deltas. The review feedback points out several structural discrepancies between the pseudo-code definitions in the RFC and the actual Rust implementation in the codebase (such as in Registry, Node, InputSlot, RegistryDelta, and Delta), providing concrete code suggestions to align them.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread node-graph/rfcs/document-format.md
Comment thread node-graph/rfcs/document-format.md
Comment thread node-graph/rfcs/document-format.md
Comment thread node-graph/rfcs/document-format.md

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 1 file

Confidence score: 3/5

  • In node-graph/rfcs/document-format.md, the library::exported_nodes_ts sidecar under Registry.attributes overlaps with the ChangeDocumentAttribute write path, so a normal attribute edit could silently corrupt the LWW basis for exported nodes updates — reserve/protect that key (or move it out of generic attributes) before merging.
  • In node-graph/rfcs/document-format.md, RegistryDelta::RegisterPeer has no reverse form even though Delta.reverse is required, which leaves this operation impossible to represent as a valid reversible delta — add a reverse variant or relax/document reverse semantics for non-reversible ops before merge.
  • In node-graph/rfcs/document-format.md, inputs_attributes: Vec<Attributes> has no invariant tying it to inputs: Vec<InputSlot>, so mismatched lengths could silently desynchronize node inputs and their metadata — document and enforce a length invariant (including validation on AddNode and input-count changes).
  • In node-graph/rfcs/document-format.md, the RFC’s delta field names/types diverge from graph-storage (node_id/network vs id, usize vs u32, delta_type vs kind), which increases integration and maintenance risk from spec/implementation drift — align the RFC to current code (or add an explicit mapping note) before merging.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread node-graph/rfcs/document-format.md Outdated
Comment thread node-graph/rfcs/document-format.md Outdated
Comment thread node-graph/rfcs/document-format.md Outdated
Comment thread node-graph/rfcs/document-format.md Outdated
Comment thread node-graph/rfcs/document-format.md

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file

Confidence score: 3/5

  • In node-graph/rfcs/document-format.md, the proposed add-wins OR-set for the resource source chain is missing tombstone/observation tracking, so the current Vec add/remove behavior is order-dependent rather than add-wins; merging as-is risks baking incorrect CRDT semantics into the spec and causing divergent behavior across implementations — define the required tombstone or causal-observation mechanism and update the RFC semantics before merging.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread node-graph/rfcs/document-format.md Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread node-graph/rfcs/document-format.md Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant