[Graphite MQ] Draft PR GROUP:spec_90ff7c (PRs 23) - #28
Closed
graphite-app[bot] wants to merge 1 commit into
Closed
Conversation
…R-13656) (#23) Split out of #21 so the contested part can be argued separately. **This half changes no parsing behaviour at all** — every value accepted before is accepted now, every value rejected before is still rejected, and the request body is byte-identical to master across plain text, quoted JSON, `null`, objects, arrays, empty and numeric input. Only the messages change. ## 1. Rejection messages name the problem, not the first character ``` vf agent update --llm 'gpt-4' invalid value for --llm: error unmarshalling json response body: invalid character 'g' looking for beginning of value ``` There is no response body — the request was never sent. Worse, **valid JSON of the wrong shape got the same treatment**: `--llm '[1,2]'` parses perfectly, and the CLI insisted it was not valid JSON, sending the reader to re-check syntax that was never wrong. The decoder's own explanation — the only thing that knows which Go type was expected — was discarded. Now shape mismatch is named separately from syntax error, an example of the right shape is derived from the destination, and the decoder's message is surfaced minus its misleading prefix: ``` invalid value for --llm: the value is valid JSON but not the shape this flag expects you passed: [1,2] expected shape: --llm '{"key":"value"}' decoder reported: cannot unmarshal array into Go value of type map[string]json.RawMessage pass null to clear the field: --llm null ``` The example is **derived** rather than fixed because a hardcoded one told array-valued flags to pass an object — following the CLI's own advice reproduced the identical error. A test now takes the suggestion and feeds it back in. For the nullable string fields this is the entire fix from the caller's side: ``` invalid value for --instructions: expected a JSON value you passed: You are a support agent. the value is not valid JSON expected shape: --instructions '"your text here"' decoder reported: invalid character 'Y' looking for beginning of value ``` Following that advice works. Previously nothing said the value had to be JSON-quoted at all. ## 2. Flag errors reach the same renderer as everything else They never did — they returned raw from the generated `RunE` and printed via a bare `Fprintln`, so a mistyped flag got one unstructured line while an API 401 got a structured envelope. Backwards, since mistyped flags are far more common. `flagutil` cannot import `output` (`output` imports `flagutil`), so failures carry a typed `FlagValueError` up to `Execute`, which renders it. What gets echoed is bounded in both modes, and counted in runes. ## 3. Help shows the flag's type, not a word from its prose | | before | after | |---|---|---| | `agent update` | `--instructions Name` | `--instructions string` | | `transcript search` | `--version-param environmentAlias` | `--version-param string` | pflag reads the first back-quoted word in a usage string as the value placeholder, and our descriptions come from the spec where backticks are emphasis. 12 flags. The worst is `--version-param`, whose only legal values are `draft` and `published`. Fixed where descriptions enter cobra, so a 13th is covered automatically. `--usage` is byte-identical — it was already correct. ## 4. Agent-mode errors print exactly once Every printer in `internal/output` writes to stderr *and* returns the error, and `main.go` printed whatever `Execute` returned. `AgentModeError`'s own doc comment promised a single print, but `main.go` is generated and printed unconditionally, so the contract was never kept. stderr is now a single parseable JSON document for all three agent-mode error shapes. Pretty output is deliberately untouched: there the trailing line is the readable summary that follows a raw API body, not a duplicate. ## Footprint and verification **12 insertions, 7 deletions across five generated files.** Everything else is new non-generated files, so there is very little for `speakeasy run` to merge. - **35 tests: 20 fail against master, 15 pass on both.** The 15 are the invariants — including that `--instructions null` still sends a real JSON null. - Parsing verified identical to master on 7 input shapes. - Full suite shows the same pre-existing failures as master. ## Relationship to #21 #21 is closed in favour of this plus #24. The raw-text fallback — the part @effervescentia requested changes on — is isolated there so this can land on its own.
graphite-app
Bot
deleted the
gtmq_spec_90ff7c_1788535314616-6d5ed3c5-b98e-4934-a2c4-ea8330ff4ec8
branch
September 4, 2026 15:22
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.
This draft PR was created by the Graphite merge queue.
Trunk will be fast forwarded to the HEAD of this PR when CI passes, and the original PRs will be closed.
The following PRs are included in this draft PR: