From 9bf98f8b3937c338e44cd5777d0d1d8b995a8a29 Mon Sep 17 00:00:00 2001 From: Richie Caputo Date: Tue, 4 Aug 2026 13:03:58 -0400 Subject: [PATCH] Reject --sort-by where the API has no sortBy parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only /events accepts sortBy (enum: id, date — confirmed against the live API). Everywhere else the CLI's allow-list dropped the flag before building the request, and those endpoints return rows in insertion order, so a caller who passed --sort-by date got the oldest documents on page one with no indication anything had gone wrong. Add sortFields to the resource table and validate against it. An unknown field on events, or any --sort-by on a resource that cannot sort, now exits 2 and points at the events-first recipe. --direction is left alone: every list endpoint accepts it, it just reverses insertion order rather than date order. Closes #4 Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 18 ++++++++++ internal/cli/app.go | 4 +++ internal/cli/cli_test.go | 69 +++++++++++++++++++++++++++++++++++++++ internal/cli/errors.go | 15 +++++++++ internal/cli/flags.go | 2 +- internal/cli/handlers.go | 7 ++++ internal/cli/help.go | 12 ++++++- internal/cli/resources.go | 51 +++++++++++++++++++++++++++++ 8 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 internal/cli/errors.go diff --git a/README.md b/README.md index ff6b9d0..15b2fb5 100644 --- a/README.md +++ b/README.md @@ -206,6 +206,24 @@ Endpoint-specific filters are also available where relevant: Companies are the one common exception where Quartr uses `ids` instead of `companyIds`. This CLI maps `--company-ids` to `ids` for `companies list`. +## Sorting + +`--sort-by` is only implemented by `/events`, where the accepted fields are `id` and `date`. Every other list endpoint rejects the parameter outright, so the CLI now fails with exit code 2 instead of dropping the flag: + +```bash +quartr transcripts list --tickers AAPL --sort-by date +# --sort-by is not supported by `quartr transcripts list` ... (exit 2) +``` + +This matters because the failure used to be invisible: document endpoints return rows in insertion order, so the newest filings and calls are simply absent from the first page. Sort events first, then fetch documents by event id: + +```bash +quartr events list --tickers AAPL --sort-by date --direction desc --limit 5 +quartr transcripts list --event-ids 406161 +``` + +`--direction asc|desc` is accepted by every list endpoint, but it reverses insertion order, not date order. + ## Downloads Download commands first retrieve metadata, then download the URL field from the response. diff --git a/internal/cli/app.go b/internal/cli/app.go index 2260f59..7522f3d 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -67,6 +67,10 @@ func Run(args []string, out, errOut io.Writer) int { return 0 } fmt.Fprintln(errOut, err) + var usage *usageError + if errors.As(err, &usage) { + return 2 + } return 1 } return 0 diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index b2c8708..26c8a6d 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -71,6 +71,75 @@ func TestDownloadAllowsFlagsAfterID(t *testing.T) { } } +func TestSortByRejectedOnUnsortableResource(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + t.Fatalf("expected no request, got %s", r.URL) + })) + defer srv.Close() + + var out, errOut bytes.Buffer + code := Run([]string{"--no-config", "--api-key", "secret", "--base-url", srv.URL, + "transcripts", "list", "--tickers", "AAPL", "--sort-by", "date"}, &out, &errOut) + if code != 2 { + t.Fatalf("expected usage exit code 2, got %d; stderr=%s", code, errOut.String()) + } + for _, want := range []string{"--sort-by is not supported", "quartr events list", "--event-ids"} { + if !strings.Contains(errOut.String(), want) { + t.Fatalf("expected stderr to mention %q, got %s", want, errOut.String()) + } + } +} + +func TestSortByRejectsUnknownFieldOnEvents(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + t.Fatalf("expected no request, got %s", r.URL) + })) + defer srv.Close() + + var out, errOut bytes.Buffer + code := Run([]string{"--no-config", "--api-key", "secret", "--base-url", srv.URL, + "events", "list", "--sort-by", "title"}, &out, &errOut) + if code != 2 { + t.Fatalf("expected usage exit code 2, got %d; stderr=%s", code, errOut.String()) + } + if !strings.Contains(errOut.String(), "supported sort fields: id, date") { + t.Fatalf("expected supported field list, got %s", errOut.String()) + } +} + +func TestSortByForwardedOnEvents(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.URL.Query().Get("sortBy"); got != "date" { + t.Fatalf("expected sortBy=date, got %q", got) + } + if got := r.URL.Query().Get("direction"); got != "desc" { + t.Fatalf("expected direction=desc, got %q", got) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":[{"id":1,"title":"Q4 2025"}],"pagination":{"nextCursor":null}}`)) + })) + defer srv.Close() + + var out, errOut bytes.Buffer + code := Run([]string{"--no-config", "--api-key", "secret", "--base-url", srv.URL, + "events", "list", "--sort-by", "date", "--direction", "desc"}, &out, &errOut) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr=%s", code, errOut.String()) + } +} + +func TestSortByRejectedOnChildList(t *testing.T) { + var out, errOut bytes.Buffer + code := Run([]string{"--no-config", "--api-key", "secret", "--base-url", "http://127.0.0.1:0", + "reports", "pages", "123", "--sort-by", "date"}, &out, &errOut) + if code != 2 { + t.Fatalf("expected usage exit code 2, got %d; stderr=%s", code, errOut.String()) + } + if !strings.Contains(errOut.String(), "quartr reports pages") { + t.Fatalf("expected command name in message, got %s", errOut.String()) + } +} + func TestListAllFollowsPagination(t *testing.T) { requests := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/cli/errors.go b/internal/cli/errors.go new file mode 100644 index 0000000..41bee75 --- /dev/null +++ b/internal/cli/errors.go @@ -0,0 +1,15 @@ +package cli + +import "fmt" + +// usageError marks a failure caused by the command line the user typed +// rather than by the API or the network. Run maps it to exit code 2 so +// scripts can tell "you asked for something impossible" apart from "the +// request failed". +type usageError struct{ msg string } + +func (e *usageError) Error() string { return e.msg } + +func usagef(format string, args ...any) error { + return &usageError{msg: fmt.Sprintf(format, args...)} +} diff --git a/internal/cli/flags.go b/internal/cli/flags.go index e322cea..26a71a7 100644 --- a/internal/cli/flags.go +++ b/internal/cli/flags.go @@ -119,7 +119,7 @@ func addListFlags(fs *flag.FlagSet, lf *listFlags) { fs.StringVar(&lf.expand, "expand", "", "comma-separated fields to expand, e.g. event") fs.StringVar(&lf.states, "states", "", "comma-separated live states") fs.StringVar(&lf.transcriptVersion, "transcript-version", "", "live transcript stream version, e.g. 1.7") - fs.StringVar(&lf.sortBy, "sort-by", "", "sort field for endpoints that support it") + fs.StringVar(&lf.sortBy, "sort-by", "", "sort field; only `events list` supports it (id, date)") fs.StringVar(&lf.levels, "levels", "", "comma-separated chapter levels") } diff --git a/internal/cli/handlers.go b/internal/cli/handlers.go index 558bb6b..924bddb 100644 --- a/internal/cli/handlers.go +++ b/internal/cli/handlers.go @@ -123,6 +123,9 @@ func (a *app) listResource(r resource, args []string) error { if fs.NArg() > 0 { return fmt.Errorf("unexpected arguments: %s", strings.Join(fs.Args(), " ")) } + if err := validateSortBy(r, lf.sortBy); err != nil { + return err + } if lf.all && !flagWasPassed(args, "limit") { lf.limit = 500 } @@ -207,6 +210,10 @@ func (a *app) childListResource(r resource, pathTpl string, allowed paramSet, ar if fs.NArg() != 1 { return fmt.Errorf("usage: quartr %s %s ", r.name, child) } + if strings.TrimSpace(lf.sortBy) != "" { + return usagef("--sort-by is not supported by `quartr %s %s`; rows are returned in document order", + r.name, child) + } if lf.all && !flagWasPassed(args, "limit") { lf.limit = 500 } diff --git a/internal/cli/help.go b/internal/cli/help.go index 887ea31..5427e62 100644 --- a/internal/cli/help.go +++ b/internal/cli/help.go @@ -98,7 +98,17 @@ Common list flags: --limit N page size, max 500 --all follow pagination.nextCursor --fields a,b,c output fields for table/csv - +`) + if r.listPath != "" { + fmt.Fprintf(a.out, "\nSorting:\n") + if len(r.sortFields) > 0 { + fmt.Fprintf(a.out, " --sort-by %s [--direction asc|desc]\n", strings.Join(r.sortFields, "|")) + } else { + fmt.Fprintf(a.out, " --sort-by is rejected here (the endpoint has no sortBy parameter).\n %s\n", + strings.ReplaceAll(sortRecipe(r), "\n", "\n ")) + } + } + fmt.Fprint(a.out, ` Examples: `) switch r.name { diff --git a/internal/cli/resources.go b/internal/cli/resources.go index bdf42b9..12d1cc7 100644 --- a/internal/cli/resources.go +++ b/internal/cli/resources.go @@ -1,5 +1,10 @@ package cli +import ( + "fmt" + "strings" +) + type paramSet []string func params(xs ...string) paramSet { @@ -42,6 +47,10 @@ type resource struct { listParams paramSet getParams paramSet summaryParams paramSet + // sortFields lists the values the endpoint accepts for sortBy. Empty + // means the endpoint has no sortBy parameter at all, which the CLI + // reports instead of dropping the flag on the floor. + sortFields paramSet } var ( @@ -69,6 +78,11 @@ var ( summaryParams = params("length", "plain") getExpandParams = params("expand") getLiveParams = params("transcriptVersion") + + // eventSortFields mirrors the enum the API reports when sortBy is + // invalid: "sortBy must be one of the following values: id, date". + // /events is the only list endpoint that accepts the parameter. + eventSortFields = params("id", "date") ) var resources = map[string]resource{ @@ -85,6 +99,7 @@ var resources = map[string]resource{ summaryPath: "/events/{id}/summary", listParams: mergeParams(baseListParams, params("typeIds", "sortBy")), summaryParams: summaryParams, + sortFields: eventSortFields, }, "documents": { name: "documents", @@ -177,3 +192,39 @@ func resourceByName(name string) (resource, bool) { r, ok := resources[name] return r, ok } + +// validateSortBy rejects --sort-by on list endpoints that have no sortBy +// parameter. Forwarding it is a 400 and dropping it is worse: rows come back +// in insertion order, so a caller who trusts the flag silently reads stale +// documents off the first page. +func validateSortBy(r resource, sortBy string) error { + sortBy = strings.TrimSpace(sortBy) + if sortBy == "" { + return nil + } + if len(r.sortFields) > 0 { + if r.sortFields.allows(sortBy) { + return nil + } + return usagef("--sort-by %s is not supported by `quartr %s list`; supported sort fields: %s", + sortBy, r.name, strings.Join(r.sortFields, ", ")) + } + return usagef("--sort-by is not supported by `quartr %s list`: the Quartr endpoint has no sortBy "+ + "parameter, so rows come back in insertion order and the newest items may be missing from the "+ + "first page.\n\n%s", r.name, sortRecipe(r)) +} + +// sortRecipe is the "do this instead" paragraph shown both by the --sort-by +// error and by `quartr --help`. +func sortRecipe(r resource) string { + const directionNote = "`--direction asc|desc` is accepted here, but it reverses insertion order, not date order." + if !r.listParams.allows("eventIds") { + return "Only `quartr events list` supports --sort-by (fields: " + + strings.Join(eventSortFields, ", ") + ").\n" + directionNote + } + return fmt.Sprintf(`Sort events first, then fetch by event id: + quartr events list --tickers AAPL --sort-by date --direction desc --limit 5 + quartr %s list --event-ids + +%s`, r.name, directionNote) +}