fix: honor export --format json; accept neighbors/query positionals - #24
Conversation
…ery positionals
Two confirmed-live bugs in pm-graph v2026.7.14 (pm-graph-1fr9):
G2 (export --format json emits TOON, not JSON):
The `pm-graph export` handler ignored `--format` entirely and always
returned `{ ok, graph }`, which the host renders as TOON by default (JSON
only with --json). Now the handler parses `--format` (both `--format <fmt>`
and `--format=<fmt>`); when present it renders the graph via `renderExport`
and returns the raw payload wrapped in a `__pmGraphRawOutput` marker. A
registered `output_format` service override — chained (multiple overrides
coexist by design) and strictly scoped to `pm-graph export` + the marker —
unwraps the marker so the host writes the raw string straight to stdout.
`--format json` now emits a valid JSON Graph document (JSON.parse(stdout)
succeeds); `cypher|mermaid|dot|graphml|plantuml` work via the same path.
Default (no --format) and --json behaviour is unchanged. Adds the
`services` capability to manifest.json for the service override.
G1 (neighbors/query reject documented positional at contract layer):
`pm pm-graph neighbors <id>` / `pm pm-graph query "<cypher>"` failed with
"Too many arguments for extension command". Root cause: these handlers
threw plain `Error` (no `exitCode`), which pm's `runCommandHandler` swallows
(handled=false), falling through to `runRequiredExtensionCommand` ->
`validateDynamicExtensionCommandArgs`; with no `arguments` declared,
maxCount=0 and the positional triggered the contract rejection. Sibling
commands `path`/`impact` work because they throw `CommandError` (numeric
exitCode), which `runCommandHandler` rethrows so it propagates cleanly. Fix:
declare the positional arguments on `neighbors` (`node-id`) and `query`
(`cypher-query`, variadic to preserve the multi-token join behaviour) AND
throw `CommandError` so errors propagate to the clear "Neo4j is not
configured" message instead of a contract/usage rejection.
Tests: 11 new integration tests drive the real pm CLI contract layer
against a throwaway workspace, asserting --format json validity, default
/--json regression, neighbors/query reaching "Neo4j is not configured"
(not "Too many arguments"), missing-arg contract enforcement, and that
path/impact still accept positionals. 66 tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
WalkthroughUpdates Changespm-graph command behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant pm_graph_export
participant loadGraphForContext
participant renderExport
participant output_format
CLI->>pm_graph_export: invoke export with --format
pm_graph_export->>loadGraphForContext: load graph
pm_graph_export->>renderExport: render selected format
pm_graph_export->>output_format: return raw-output marker
output_format-->>CLI: print rendered graph
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
@greptileai review |
|
@coderabbitai full review |
|
/gemini review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 4 minutes. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses two bugs in pm-graph: it ensures that pm pm-graph export --format json outputs valid JSON instead of TOON by utilizing a new output_format service override, and it resolves contract-layer positional argument rejections for the neighbors and query commands by declaring their arguments and throwing CommandErrors. The changes also introduce comprehensive integration tests. Feedback on the new tests highlights two issues: spawnSync does not throw on missing executables, which breaks the pmAvailable check when pm is absent, and using __dirname as a fallback in ES modules will cause a ReferenceError on Node.js versions older than v20.11.0.
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.
There was a problem hiding this comment.
Code Review
This pull request addresses two bugs in pm-graph: it ensures that pm pm-graph export --format json correctly outputs valid JSON on stdout by utilizing a new output_format service override, and it resolves a contract-layer positional argument rejection for the neighbors and query commands by declaring their arguments and throwing CommandError instead of generic errors. Additionally, it updates the manifest, types, documentation, and adds comprehensive integration tests. The review feedback highlights a bug in the flag-parsing helper readFlagStringValue where a flag without a value (e.g., --format) followed by another flag (e.g., --json) would incorrectly consume the subsequent flag as its value, and provides code suggestions to fix this behavior.
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.
Greptile SummaryThis PR fixes pm-graph export formatting and positional CLI handling. The main changes are:
Confidence Score: 4/5The export formatting and positional contract changes otherwise look covered by the added tests. The changed query parsing path still mishandles a valid split Cypher token: src/index.ts
 T-Rex LogsWhat T-Rex did
Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/index.ts:2421
**Help Flag Becomes Query Control**
When a split variadic Cypher query contains `-h`, such as `WITH 1 AS h RETURN -h`, this branch runs before the query text is rebuilt. The command returns help output instead of preserving the token and running the query, so callers do not get the expected Neo4j result or configuration error.
Reviews (19): Last reviewed commit: "fix(graph): preserve typed CommandError ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes
Confidence Score: 5/5This looks safe to merge. No blocking issues were found in the changed code. The raw-output override is scoped to No files need attention.
T-Rex LogsWhat T-Rex did
Important Files Changed
Reviews (2): Last reviewed commit: "fix(export/neighbors/query): honor --for..." | Re-trigger Greptile |
…flag values
Addresses review findings on the neighbors/query positional + --format fix:
- Greptile P1: the variadic Cypher positional joined ALL context.args, so a
trailing '--json' became part of the query string ('MATCH ... LIMIT 10
--json'), which Neo4j rejects as a syntax error. Filter out '--'-prefixed
flag tokens before joining. Same guard applied to 'neighbors <id>'.
- Gemini (medium): readFlagStringValue consumed a following flag as a value
('--format --json' -> format='--json'). Treat a next-token starting with
'--' (or absent) as 'no value' and return null so the default applies.
- Gemini (high/medium) in the contract test: spawnSync does not throw on
ENOENT, so the pm-availability probe now checks result.error/status instead
of a try/catch; and __dirname (undefined in ESM) is replaced with a
fileURLToPath(import.meta.url) derivation that works on all Node versions.
66/66 tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptileai review |
|
@coderabbitai full review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request resolves two main issues: G2, which enables pm pm-graph export --format json to output valid JSON to stdout instead of a TOON summary, and G1, which fixes contract-level argument rejections for neighbors and query commands by declaring positional arguments and propagating CommandErrors correctly. It introduces an output_format service override to handle raw outputs, adds robust flag parsing, and includes comprehensive integration tests. Feedback on the changes suggests refining the flag parser helper to prevent swallowing short flags starting with a single hyphen.
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.
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 18 minutes. |
… --long
Gemini follow-up: readFlagStringValue only skipped '--'-prefixed values, so a
short flag (e.g. '-j') after '--format' would still be swallowed. No valid
flag value or node id starts with '-', so guard on startsWith('-') for the
flag-value lookahead and the query/neighbors arg filters alike.
66/66 tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eserve Cypher Greptile P1 follow-up: filtering single-dash tokens from the variadic query positional dropped valid Cypher negative literals (e.g. 'n.score < -1'). Cypher never uses '--'-prefixed tokens, so strip only two-dash flags (--json/--help) from the positionals to preserve query text. The short-flag guard stays on readFlagStringValue (--format), whose values never start with '-'. 66/66 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptileai review |
|
@coderabbitai full review |
|
/gemini review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 55 minutes. |
There was a problem hiding this comment.
Code Review
This pull request addresses two bugs in pm-graph: it fixes the export --format json command to output valid JSON on stdout instead of TOON format by introducing an output_format service override, and it resolves positional argument rejection at the contract layer for the neighbors and query commands by declaring their arguments and throwing CommandErrors. It also adds comprehensive integration tests. The review feedback highlights a performance bottleneck where the loadGraph helper function inefficiently spawns N+1 parallel child processes; it is recommended to replace it with loadGraphForContext to fetch the graph in a single call.
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.
Gemini follow-up: loadGraph spawned N+1 child processes (one list-all plus a per-item 'pm deps <id>' call) to build the graph, which can exhaust OS process limits on large workspaces. loadGraphForContext already builds the same graph from a single 'list-all --json --include-body' call (the payload carries dependencies[]/blocked_by), so switch all four command call sites to it and drop the now-unused loadGraph + dependencyRows helpers. 66/66 tests pass (graph output unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptileai review |
|
@coderabbitai full review |
|
/gemini review |
|
@coderabbitai full review |
|
/gemini review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 38 minutes. |
There was a problem hiding this comment.
Code Review
This pull request addresses two bugs in the pm-graph extension: it ensures pm pm-graph export --format json outputs valid JSON on stdout by utilizing a new output_format service override, and it resolves contract-layer positional argument rejections for the neighbors and query commands by declaring their arguments and throwing CommandErrors. Additionally, unused helper functions were cleaned up, and comprehensive integration tests were added. The review feedback suggests preserving the original exitCode when catching and wrapping errors that are already instances of CommandError in several catch blocks across src/index.ts to ensure accurate error propagation.
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.
…tive -h help override - Gemini: the sync graph-load catch now re-throws an existing CommandError as-is instead of re-wrapping it and flattening exitCode to GENERIC_FAILURE. - Greptile/CodeRabbit '-h in split query': verified the pm HOST contract layer intercepts a standalone '-h'/'--help' token and prints command help BEFORE the handler runs, so an extension-level help override is dead code — reverted it. The documented usage quotes the query into one arg (covered by the dash-token test), where the host never sees a standalone '-h'. Unquoted split queries with a bare '-h' are a host-level limitation, not extension-controllable. 67/67 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Round-7 responses (commit ca81bee)
67/67 tests pass. |
|
@greptileai review |
|
@coderabbitai full review |
|
/gemini review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 16 minutes. |
There was a problem hiding this comment.
Code Review
This pull request addresses two bugs in the pm-graph extension: it ensures pm-graph export --format json outputs valid JSON on stdout by registering an output_format service override, and it declares positional arguments for neighbors and query commands to prevent contract-layer argument validation failures. Additionally, integration tests were added, and unused helper functions were removed. The review feedback suggests preserving the original CommandError instances and their exit codes when catching errors in the export command handlers, rather than flattening them to a generic failure code.
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.
Gemini: the two 'Export failed' catch blocks flattened an already-typed CommandError from loadGraphForContext to GENERIC_FAILURE, masking its exit code (same issue fixed in sync). Add 'if (err instanceof CommandError) throw err;' guards so all three graph-load catches preserve the original error. 67/67 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Round-8 responses (commit 082be73)
67/67 tests pass. |
|
@greptileai review |
|
@coderabbitai full review |
|
/gemini review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 47 seconds. |
There was a problem hiding this comment.
Code Review
This pull request resolves two key bugs in the pm-graph extension. First, it updates the pm pm-graph export command to support the --format flag (including json, mermaid, dot, etc.) and ensures that raw formatted strings are written directly to stdout using a new output_format service override. Second, it fixes positional argument rejection for pm pm-graph neighbors and pm pm-graph query by declaring their arguments in the command registration and throwing CommandError instead of plain Error to prevent the host from swallowing errors and falling through to a contract rejection. Feedback is provided regarding a potential bug in pm-graph query where an unquoted query containing -h (e.g., RETURN -h) would prematurely trigger the help screen due to the generic hasHelpFlag check.
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.
Final note on the unquoted-
|
1 participant
Problem
Two confirmed-live bugs in
pm-graphv2026.7.14 (pm-graph-1fr9):pm pm-graph export --format jsonis accepted but emits TOON (ok: true ...), not JSON. Only--jsonyields JSON.pm pm-graph neighbors <node-id>andpm pm-graph query "<cypher>"reject their documented positional arg at the contract layer withToo many arguments for extension command, while sibling commandspath <from> <to>andimpact <id>accept positionals.Root cause
G2: The
pm-graph exporthandler ignored--formatentirely and always returned{ ok, graph }, which the host renders as TOON by default (JSON only with--json).G1: This is NOT primarily a missing
arguments:declaration.path/impactwork because they throwCommandError(numericexitCode), which pm'srunCommandHandlerrethrows so the error propagates cleanly.neighbors/querythrew plainError(noexitCode), whichrunCommandHandlerswallows (handled: false), causing a fall-through torunRequiredExtensionCommand→validateDynamicExtensionCommandArgs; with noargumentsdeclared,maxCount = 0and the positional triggered theToo many argumentscontract rejection. (The file already documents this contract at the top ofsrc/index.ts.)Fix
G2: The export handler now parses
--format(both--format <fmt>and--format=<fmt>). When present, it renders the graph via the existingrenderExportand returns the raw payload wrapped in a__pmGraphRawOutputmarker. A registeredoutput_formatservice override — chained (multiple overrides coexist by design) and strictly scoped topm-graph export+ the marker — unwraps the marker so the host writes the raw string straight to stdout.--format jsonnow emits a valid JSON Graph document (JSON.parse(stdout)succeeds);cypher|mermaid|dot|graphml|plantumlwork via the same path. Default (no--format) and--jsonbehaviour are unchanged. Adds theservicescapability tomanifest.json.G1: Declare the positional arguments on
neighbors(node-id) andquery(cypher-query, variadic to preserve the multi-tokenjoin(" ")behaviour), AND throwCommandError(withexitCode) instead of plainErrorso errors propagate cleanly to the "Neo4j is not configured" message instead of a contract/usage rejection.Test evidence
11 new integration tests (
test/export-and-contract.test.ts) drive the real pm CLI contract layer against a throwawaypm initworkspace with the locally-built package installed. All 66 tests pass (npm test):pm pm-graph export --format json→ stdout is valid JSON (JSON.parsesucceeds, hasgraph.nodes/graph.edges, nookenvelope).pm pm-graph export(no--format) → unchanged TOONok: truesummary.pm pm-graph export --json→ unchanged JSON{ ok, graph }envelope.pm pm-graph export --format mermaid→ rawgraph TDdiagram (no TOON prefix).pm pm-graph export --format svg→ clean USAGEUnknown --format "svg".pm pm-graph neighbors TASK-42→ exits non-zero withNeo4j is not configured(NOTToo many arguments).pm pm-graph query "MATCH (n) RETURN n LIMIT 5"→ exits non-zero withNeo4j is not configured(NOTToo many arguments).pm pm-graph neighbors(no arg) → contractMissing required argument node-id.pm pm-graph query "CREATE (n) RETURN n"→Blocked destructive Cypher keyword "CREATE"(clean, not contract rejection).pm pm-graph path/impactstill accept positionals and reach the handler.npm run build,npm run typecheck, andnpm run changelog:checkpass.pm item
pm-graph-1fr9
Summary by cubic
Fixes
pm-graphCLI handling:pm pm-graph export --format jsonnow writes raw, valid JSON to stdout, andneighbors/queryaccept their positionals with clear errors. Also switches graph loading to a single-call path to avoid N+1 subprocesses. Addresses pm itempm-graph-1fr9.Bug Fixes
--format: Parse both--format <cypher|mermaid|dot|json|graphml|plantuml>and--format=<...>, last-wins if repeated, reject missing/invalid values, render via the existing exporter, and write the raw payload via anoutput_formatservice override. Default output (no--format) and--jsonare unchanged. Adds"services"tomanifest.json.neighbors <node-id>andquery <cypher-query...>positionals. Strip only the exact flags--jsonand--helpfrom positionals so they aren’t absorbed; preserve all Cypher dash tokens (e.g.--,-->,<--,-1). Guard--formatparsing from swallowing short or following flags and keep last-wins.CommandError(and its exit code) in export and sync paths instead of flattening to a generic failure.Refactors
loadGraphForContext(singlelist-all --include-bodycall) across graph commands; remove the N+1 subprocess path.output_formatoverride (no behavior change).Written for commit 082be73. Summary will update on new commits.