Skip to content

fix(cli): render paginated API results in text mode - #3984

Merged
lyingbug merged 1 commit into
Tencent:mainfrom
dvd233:fix/cli-pagination-text-output
Oct 7, 2026
Merged

lyingbug merged 1 commit into
Tencent:mainfrom
dvd233:fix/cli-pagination-text-output

Conversation

@dvd233

@dvd233 dvd233 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Description

weknora api --paginate --format text fetches and merges the pages, then fails with FormatOptions.Emit: cannot emit text mode as JSON; caller must render human-readable separately. The same failure occurs when text is selected through WEKNORA_FORMAT=text.

This change adds the missing text dispatch after aggregation. Text emits one bare {data,total} JSON object followed by a newline. JSON and NDJSON keep their existing emitter paths, and the merged json.RawMessage values retain their precision. Output errors use the existing local.file_io classification.

The paginated GET path also rejects resolved text mode with --jq before making HTTP requests. This covers text selected through the environment; an explicit JSON or NDJSON flag still takes precedence and applies its filter. API help and agent-help now identify JSON as the default and describe text aggregation.

Only cli/cmd/api/api.go and the new cli/cmd/api/paginate_text_test.go change. The shared FormatOptions.Emit text guard, pagination loop, raw fallback, and non-GET/non-paginated behavior stay unchanged.

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • 📚 Documentation update
  • 🎨 Refactor
  • ⚡ Performance improvement
  • 🧪 Test
  • 🔧 Configuration / Build / CI

Related Issue

This is independent of #3983. That PR propagates response-body read errors; this PR handles successfully read and aggregated text results. Both patches apply cleanly in either order, and the combined tree passes the full CLI tests, race checks, vet, and build. Neither patch depends on the other.

Testing

Validated locally on Linux/amd64 using Go 1.26.8 from the separate cli/ module:

  • go test -count=1 ./...
  • go test -race -count=1 ./...
  • go vet ./...
  • go build ./...
  • go test ./cmd/api -run TestAPI_Paginate -count=20
  • golangci-lint run --new-from-rev=bccb4b151bae403508da77fbb174efc79dc47c1a ./... (v2.14.0, zero issues)
  • Both CLI wire-vocabulary and secret-scan scripts
  • go mod verify, gofmt/gofumpt checks, and git diff --cached --check

The identical regression file has 10 expected failing cases and 16 passing compatibility cases on baseline bccb4b151bae403508da77fbb174efc79dc47c1a; all 26 pass with the fix. It exercises the real root command and SDK against local HTTP fixtures, including single/multiple/empty pages, format precedence, jq, raw fallback, non-GET behavior, large numeric values, and body/newline writer errors.

Separately built baseline and fixed binaries were exercised in 22 cases each, including a real PTY. Previously successful machine/raw outputs remain byte-identical. Environment-selected text with either a valid or malformed jq expression now exits 2 with empty stdout and no HTTP calls. The same 22 cases pass with both patches applied.

Two independent coding-agent reviews approved this exact patch after rerunning their own CLI, transport, writer-error, and compatibility probes. Hosted source-bound validation passed on this exact feature commit, including the strict 10-fail/16-pass baseline and 26-pass fixed checks, full CLI tests/race/vet/build, and repository scripts.

Upstream validation also passed: CLI build, race tests and vet on Linux, macOS and Windows, plus the Go Lint workflow.

Scope: the full default CLI suite includes acceptance contracts. Root backend/frontend tests, standalone SDK tests, and live-server acceptance_e2e tests were not run. Fixtures use synthetic local data and no real service credentials. Environment scrubbing and disabled module downloads do not imply kernel-level network isolation.

Checklist

  • Diff whitespace checks pass against the pinned main baseline
  • Changed source files are formatted
  • Targeted tests for the changed package pass
  • Diff-scoped lint passes
  • Full CLI-module checks pass; broader suites are listed above
  • Self-reviewed by coding agents, including two independent reviews
  • Added tests covering the change
  • Updated the relevant API help and agent-help
  • Breaking changes documented (none)

AI assistance: coding agents prepared the patch, tests, and reviews. No personal human review is claimed.

@dvd233
dvd233 marked this pull request as ready for review October 6, 2026 08:13
Copilot AI balanced review requested due to automatic review settings October 6, 2026 08:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lyingbug
lyingbug merged commit 4a0c7fa into Tencent:main Oct 7, 2026
6 checks passed
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.

3 participants