Skip to content

Add support for opening GDD documents from the CLI#4262

Merged
TrueDoctor merged 6 commits into
masterfrom
gcli-gdd
Jun 21, 2026
Merged

Add support for opening GDD documents from the CLI#4262
TrueDoctor merged 6 commits into
masterfrom
gcli-gdd

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 adds support for the new .gdd archive format in the Graphene CLI, including a new command to extract legacy .graphite documents and updating the preprocessor to resolve resources via a custom resolver. The review feedback highlights a compilation error due to calling eq_ignore_ascii_case on &OsStr, suggests returning a Result instead of panicking in the CLI, recommends directly awaiting a future instead of using block_on inside an async context, and advises using &dyn Fn instead of &impl Fn to prevent monomorphization bloat in recursive preprocessor functions.

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/graphene-cli/src/main.rs
Comment thread node-graph/graphene-cli/src/main.rs
Comment thread node-graph/graphene-cli/src/main.rs Outdated
Comment thread node-graph/preprocessor/src/lib.rs Outdated
Comment thread node-graph/preprocessor/src/lib.rs 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.

3 issues found across 6 files

Confidence score: 3/5

  • In node-graph/graphene-cli/src/main.rs, calling futures::executor::block_on inside #[tokio::main] can stall or deadlock async execution paths, which could make the CLI hang under certain workloads — replace block_on(...) with direct .await before merging.
  • In node-graph/graphene-cli/src/main.rs, newly added expect(...) calls turn recoverable I/O/parse/preprocess failures into hard panics, so users may see abrupt process aborts instead of actionable errors — switch these to ? with contextual error mapping to preserve CLI reliability.
  • Also in node-graph/graphene-cli/src/main.rs, the "write write" typo and unnecessary new_path.clone() are low-risk but reduce clarity and maintainability; the note that main returns Result suggests this path can be cleaned up with normal error propagation — fix the message and remove the clone during final polish.

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

Re-trigger cubic

Comment thread node-graph/graphene-cli/src/main.rs Outdated
Comment thread node-graph/graphene-cli/src/main.rs Outdated
Comment thread node-graph/graphene-cli/src/main.rs 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.

1 issue found across 4 files (changes from recent commits).

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

Re-trigger cubic

Comment thread node-graph/graphene-cli/src/main.rs
@TrueDoctor TrueDoctor enabled auto-merge June 21, 2026 15:55
@TrueDoctor TrueDoctor disabled auto-merge June 21, 2026 16:16
@TrueDoctor TrueDoctor merged commit b45cc31 into master Jun 21, 2026
1 check passed
@TrueDoctor TrueDoctor deleted the gcli-gdd branch June 21, 2026 16:17
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.

2 participants