fix(output): valid JSON on stdout and numeric toObjectId parsing#56
fix(output): valid JSON on stdout and numeric toObjectId parsing#56piekstra wants to merge 3 commits into
Conversation
Automated PR ReviewReviewed commit: Summary
go:implementation-tests (1 finding)Major -
|
| Field | Value |
|---|---|
| Model | claude-sonnet-4-6 |
| Reviewers | documentation:docs, go:implementation-tests, policies:conventions |
| Engine | claude_cli · claude-sonnet-4-6 |
| Reviewed by | cr · piekstra-dev |
| Duration | 2m 36s wall · 2m 35s compute |
| Cost | ~$0.32 (est.) |
| Tokens | 19 in / 5.0k out |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost | Duration |
|---|---|---|---|---|---|---|---|
| orchestrator-selection | claude-sonnet-4-6 | 3 | 124 | 3.3k | 4.8k | ~$0.02 (est.) | 29s |
| documentation:docs | claude-sonnet-4-6 | 4 | 498 | 7.2k | 11.9k | ~$0.05 (est.) | 12s |
| go:implementation-tests | claude-sonnet-4-6 | 4 | 2.2k | 7.7k | 17.0k | ~$0.10 (est.) | 47s |
| policies:conventions | claude-sonnet-4-6 | 4 | 1.7k | 7.2k | 12.3k | ~$0.07 (est.) | 37s |
| orchestrator-rollup | claude-sonnet-4-6 | 4 | 463 | 19.9k | 14.5k | ~$0.07 (est.) | 28s |
piekstra-dev
left a comment
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR review completed with outcome: comment.
Automated PR ReviewReviewed commit: Summary
3 PR discussion threads considered. 3 summarized; 3 resolved. Completed in 2m 57s | ~$0.38 (est.) | claude-sonnet-4-6 | cr 0.4.161
Per-workstream usage
|
piekstra-dev
left a comment
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR review completed with outcome: approved.
Automated PR ReviewReviewed commit: Summary
1 PR discussion threads considered. 1 summarized; 1 resolved. Completed in 2m 34s | ~$0.42 (est.) | claude-sonnet-4-6 | cr 0.4.161
Per-workstream usage
|
piekstra-dev
left a comment
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR review completed with outcome: approved.
Automated PR ReviewReviewed commit: Summary
go:implementation-tests (1 finding)Minor -
|
| Field | Value |
|---|---|
| Model | claude-sonnet-4-6 |
| Reviewers | go:implementation-tests, policies:conventions, structure:repo-health, documentation:docs |
| Engine | claude_cli · claude-sonnet-4-6 |
| Reviewed by | cr · piekstra-dev |
| Duration | 4m 34s wall · 4m 32s compute |
| Cost | ~$0.57 (est.) |
| Tokens | 25 in / 12.6k out |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost | Duration |
|---|---|---|---|---|---|---|---|
| orchestrator-selection | claude-sonnet-4-6 | 4 | 2.9k | 7.2k | 19.8k | ~$0.12 (est.) | 52s |
| go:implementation-tests | claude-sonnet-4-6 | 4 | 4.0k | 7.2k | 18.7k | ~$0.13 (est.) | 1m 20s |
| policies:conventions | claude-sonnet-4-6 | 5 | 2.5k | 22.2k | 18.4k | ~$0.11 (est.) | 54s |
| structure:repo-health | claude-sonnet-4-6 | 4 | 2.5k | 7.2k | 16.7k | ~$0.10 (est.) | 50s |
| documentation:docs | claude-sonnet-4-6 | 4 | 257 | 7.2k | 11.9k | ~$0.05 (est.) | 8s |
| orchestrator-rollup | claude-sonnet-4-6 | 4 | 447 | 15.7k | 10.6k | ~$0.05 (est.) | 26s |
piekstra-dev
left a comment
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR review completed with outcome: comment.
…bjectId Two related JSON-correctness bugs: - #52: View.Success/Info/Print/Println wrote human-facing status and progress messages to stdout (v.Out), interleaving them with the JSON payload and breaking jq / json.load() / any machine-readable pipeline. Route all four to stderr (v.Err), matching Error and Warning which already do. stdout now carries only structured data. - #51: api.Association.ToObjectID was typed as string, but the HubSpot CRM v4 associations API returns toObjectId as a JSON number, so every non-empty `associations list` response failed with "cannot unmarshal number into Go struct field ... of type string". Change the field to json.Number (accepts numbers and strings) and use .String() at the display call site. Add tests: view_test.go asserts status messages stay on stderr and that stdout alone is valid parseable JSON; associations_test.go asserts numeric and string toObjectId both parse and re-serialize to valid JSON. Closes #52 Closes #51
…x associations test Addresses code-review feedback on the --output json (#52) and associations toObjectId (#51) fixes. - view: Print/Println were generic names routed to stderr, hiding the stderr contract at call sites. They are used only for status/progress text (audit found zero primary-output callers), so rename them to PrintStatus/PrintlnStatus and update the only callers (configcmd, forms). Condense the duplicated multi-line stderr-rationale comments to one line per method and move the prose once to the View struct doc. - view test: add TestPrimaryOutputGoesToStdout asserting primary rendered output (Table/Plain/Render) lands on stdout while status goes to stderr, complementing the existing JSON-on-stdout assertion. - associations: remove the misleading 'string toObjectId also parses' subtest and the 'json.Number accepts both' comment. json.Number only accepts a quoted string when its contents are a valid number; HubSpot's v4 API returns toObjectId as an integer, so keep just the numeric regression test (the real #51 fix).
piekstra
force-pushed
the
fix/json-output-stdout-and-associations
branch
from
ace8c03 to
dbf1c43
Compare
2 participants
Closes #52
Closes #51
Fixes two related JSON-output-correctness bugs. They live in separate, otherwise-unrelated files (the output/view layer and the associations API model), but both are about producing valid JSON, so they are bundled on one branch/PR per the issue grouping. Both fixes are small.
Summary
#52 —
--output jsonwrites status messages to stdout (corrupts JSON)Root cause: In
internal/view/view.go,View.Success,Info,Print, andPrintlnall wrote tov.Out(stdout).View.JSONalso writes to stdout, so human-facing banners (✓ Note created...), count lines (Found 3 contact(s)), and pagination hints (More results available...) were interleaved with the JSON payload on the same stream — breakingjq,json.load(), and any machine-readable pipeline.Fix: Route those four methods to
v.Err(stderr), matchingErrorandWarning, which already write to stderr. Status messages still display on the terminal (stderr is shown by default), but stdout now carries only the structured payload. This is the standard Unix split: structured data on stdout, human text on stderr. Fixed at the helper level, so it applies repo-wide to every command using theView.#51 —
associations listJSON parse error when results existRoot cause: The HubSpot CRM v4 associations API returns
toObjectIdas a JSON number (e.g.98765), butapi.Association.ToObjectIDwas typed asstring.encoding/jsoncannot unmarshal a number into a Gostring, so every non-empty response failed withjson: cannot unmarshal number into Go struct field Association.results.toObjectId of type string. Empty result sets parsed fine, which made the bug deceptive.Fix: Change the field type to
json.Number(already-imported package).json.Numberaccepts both numeric and string JSON values and re-serializes a number as a number, soassociations list -o jsonnow produces valid JSON. Updated the one display call site ininternal/cmd/associations/associations.goto use.String().Also added a
### Fixedentry toCHANGELOG.mdunder[Unreleased].Testing
All commands run from the repo root.
make build→ success (binary atbin/hspt)make test(go test -race -v ./...) → PASS, no failures across all packagesmake lint(golangci-lint run, v2.12.2) → 0 issuesgo vet ./...→ cleanNew tests:
internal/view/view_test.goTestStatusMessagesGoToStderr—Success/Info/Print/Printlnwrite to stderr, never stdoutTestStdoutIsValidJSON— with status messages emitted before/afterJSON(), stdout alone is parseable JSON (json.Unmarshalsucceeds; no banners/pagination text leak)TestErrorAndWarningGoToStderr,TestJSONOutputUnaffectedByColorFlagapi/associations_test.gotoObjectId(98765) parses successfully (the regression case)toObjectIdstill parses (back-compat)"toObjectId":98765)