Repository navigation
sdk-sync: API 2.0.0+ffb34b7 → sdk-go (minor) - #31
marketdata-sdk-sync[bot] wants to merge 4 commits into
Conversation
tests: Added tests for the API's dte parameter change (int -> string on GET /v1/options/chain), which adds a comma-separated list, a closed range, and >=/<= comparison-operator forms beside the existing single-value InDTE. Added corresponding ExpiryFilter constructors' tests — InDTEs, DTERange, MinDTE, MaxDTE — as wire-serialization tests (options_test.go), httptest-backed Chain tests (service_test.go), validation tests (0..36500, matching the API's stated bound; InDTE's own pre-existing validation is left untouched per the no-new-error-on-existing-method rule), a positive compile-corpus addition, and live reflection tests (integration/reflection_live_test.go) that derive candidates from a baseline AllExpirations call rather than asserting a locally computed day count. No production code changed; this is purely additive to the ExpiryFilter union, so no compatibility probe was needed. Left out: exact inclusive/exclusive boundary semantics (API text is internally ambiguous on this) and server tie-break behavior for list candidates, per the live-test rules. implement: Implemented the API's dte parameter (string) as four new ExpiryFilter constructors beside the unchanged InDTE: InDTEs (list), DTERange (closed range), MinDTE (>=), MaxDTE (<=), each validated (0..36500) and serialized to the documented wire form. - Doc comment: every new constructor documented in its own godoc comment with the wire form it emits. - Resource docs: docs/options/chain.md gained the four bullets plus a "By days to expiry" example; ADR-017's constructor table updated. - Wire test per form: one row/test each for InDTEs, DTERange, MinDTE, MaxDTE in options_test.go and service_test.go (phase 1), unchanged. - Validation on every construction path: all four route through the single expiryParams.validate() switch that chainPath already calls for any ExpiryFilter, so there is no second path to miss; InDTEs' empty-slice case is covered. - No coverage exclusions: 100% statement coverage, no nolint/ignore added. - Examples/docs for prior forms: InDTE and the sealed-union examples are untouched and still correct. - Documentation makes only the promises the API's own text makes (0..36500, "closest expiration", dedup) — no invented tie-break or inclusivity claim added. - InDTE(days int) keeps its original signature and its original (looser, no upper bound) validation, so no existing call starts failing differently. Left unchanged, no dte/InDTE constructor usage: integration/options_test.go, integration/catalog_live_test.go, internal/apicatalog/reachability_test.go, marketdata/stocks/format_params.go, marketdata/options/types.go, marketdata/options/format_facets_test.go, examples/optionterm/*, examples/covered-call-screener/main.go, examples/multi-asset-dashboard/main.go, README.md. Flag for the API team: its release text illustrates the comparison-operator form with ">=30, <45" (a literal "<"), but since it also says the dte syntax "matches strike and delta" (which exposes only >= and <=), MinDTE/MaxDTE here use >= and <= to match that existing grammar rather than introducing a bare "<"; worth confirming which operator set the server actually accepts. CHANGELOG.md is outside limits and not updated, per instructions; no other out-of-limits file was left wrong by this change. review fix 1: Fixed the three major findings: moved MaxDTE's live-test ceiling from sortedDTEs[1] to sortedDTEs[2] so two listed dte values sit strictly below it and distinct>=2 holds under either inclusive/exclusive reading of <=N; scoped the ExpiryFilter doc comment's "0..36500" sentence to InDTEs/DTERange/MinDTE/MaxDTE only, naming that InDTE keeps its original unbounded check; and updated docs/RESIDUALS.md's union table and dte value-range row to list the four new constructors and split InDTE's (unbounded) rule from the new forms' (0..36500) rule, matching the ADR-017 fix from the prior round. Resolved the minor note: DTERange's n<8 fallback now uses loIdx=1 (never 0, to keep excluding the unfiltered front-month value) and requires n>=5 so at least two listed dte values fall strictly between lo and hi regardless of inclusive/exclusive reading; added a dedicated skip for that subtest since it now needs more listed values than the shared List/Min/Max guard. Did not act on the question-for-a-person about whether a strict `<`/`>` dte form should be reachable — left for a person, no code change made on that basis. Re-verified: go test -race ./... green; go vet ./... and go vet -tags=integration ./integration/... clean; golangci-lint run 0 issues; all three example apps (tuitest, stockterm, optionterm) vet/test green; coverage 100.0% via sdk-sync-go-coverage. changelog: Added changelog entry under [Unreleased] / Added documenting the four new ExpiryFilter constructors (InDTEs, DTERange, MinDTE, MaxDTE) for the API's dte parameter (API release 2.0.0) supporting comma-separated lists, ranges, and >= / <= operators; InDTE(days int) works as before.
🧾 Job log — sdk-syncsdk-auto-sync job log
Changes in the API surface1. testsCoder (56 turns, US$ 2.80, 10 min 40 s):
The pipeline:
Gates:
2. implementCoder (60 turns, US$ 3.52, 5 min 33 s):
Gates:
3. review 1Reviewer (29 turns, US$ 3.38, 4 min 31 s): 3 blocking finding(s)
4. review fix 1Findings sent back to the coder:
Minor findings sent along, as optional:
Questions for a person, shown to the coder and not for it to resolve:
Coder (18 turns, US$ 1.21, 2 min 12 s):
Gates:
5. review 2Reviewer (4 turns, US$ 0.67, 58 s): approved
6. securitySecurity (3 turns, US$ 0.45, 18 s): approved
7. assessAnalyst (4 turns, US$ 0.17, 14 s) — public surface:
Computed by the pipeline: minor; changelog entry written 8. changelogCoder (8 turns, US$ 0.11, 42 s):
Gates:
Questions for a personThe internal reviewers raised these for a person to answer. They did not block the change, and the coder was not asked to resolve them.
Totals
Agent runs
Code review fix 1 — 2026-11-03 13:30:22ZOutcome: done Time: 13 min 01 s (agents 12 min 34 s, gates 27 s) 1. code review fix 1The code review asked for changes:
Coder (78 turns, US$ 1.90, 6 min 43 s):
Gates:
2. review 1Reviewer (6 turns, US$ 1.51, 2 min 17 s): 1 blocking finding(s)
3. review fix 1Findings sent back to the coder:
Minor findings sent along, as optional:
Coder (21 turns, US$ 0.66, 1 min 51 s):
Gates:
4. review 2Reviewer (4 turns, US$ 1.07, 1 min 13 s): approved
5. securitySecurity (5 turns, US$ 0.53, 30 s): approved
Code review fix 2 — 2026-11-03 14:21:14ZOutcome: done Time: 6 min 18 s (agents 6 min 05 s, gates 13 s) 1. code review fix 2The code review asked for changes:
Coder (31 turns, US$ 0.80, 4 min 29 s):
Gates:
2. review 1Reviewer (11 turns, US$ 0.94, 1 min 36 s): approved
3. securityThe pipeline:
Questions for a personThe internal reviewers raised these for a person to answer. They did not block the change, and the coder was not asked to resolve them.
CI repair 1 — 2026-11-03 14:41:23ZOutcome: done Time: 10 min 59 s (agents 10 min 33 s, gates 26 s) 1. CI fix 1The repository's CI failed:
Coder (45 turns, US$ 1.16, 5 min 05 s):
Gates:
2. review 1Reviewer (14 turns, US$ 1.12, 2 min 27 s): 2 blocking finding(s)
3. review fix 1Findings sent back to the coder:
Minor findings sent along, as optional:
Coder (16 turns, US$ 0.44, 1 min 36 s):
Gates:
4. review 2Reviewer (7 turns, US$ 0.71, 1 min 01 s): approved
5. securitySecurity (3 turns, US$ 0.28, 24 s): approved
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 44 44
Lines 3140 3191 +51
=========================================
+ Hits 3140 3191 +51 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🟡 Needs changes — Code reviewAdds five public DTE filter constructors/forms and broadens transient transport retries to connection resets and broken pipes. Surface comparison: InDTEs, DTERange, MinDTE, MaxDTE, and DTEExpr were absent before and now return ExpiryFilter; no existing public signature changed, so Version impact: minor. The live coverage does not establish the promised boundary semantics, and the new additions are not fully documented. Rating patch 4/6 (works with reservations) · proof 3/6 (partial) · overall 3/6 Before merge
FindingsShould fix
Since last review
Reviewed |
code review fix 1: Finding 1 (blocking, strict DTE comparisons unreachable): added `options.DTEExpr(expr string) ExpiryFilter`, a raw-expression escape hatch mirroring `StrikeExpr`, which passes any dte expression (including a strict `<`/`>` comparison) through verbatim as `dte=expr`; validated for non-empty only. Added wire, validation, and negcompile tests, godoc, docs/options/chain.md, docs/RESIDUALS.md and ADR-017 union-table entries, and two live reflection subtests (DTEExpr=strictAbove/strictBelow) asserting only the weak (`>=`/`<=`) bound each strict form implies, which holds under either reading of strictness and fails on the unfiltered single-expiration answer. Finding 2 (should-fix, DTERange inclusivity promise): removed the "inclusive range" claim from DTERange's doc comment and the matching chain.md bullet; both now say the range falls "between lo and hi" without asserting which end is included, since that is unresolved and the live test already only checks what holds either way. Left out: whether the server actually accepts a bare `<`/`>` operator is still unconfirmed (no live run available here); the new DTEExpr integration subtests assert only the weak bound the response itself can verify, not strict exclusion at the boundary. review fix 1: [major] Dropped the `DTEExpr=strictAbove` live subtest (integration/reflection_live_test.go), which sent a bare `>N` — a form neither the release text nor the SDK's own StrikeExpr/MinDTE/MaxDTE documentation states the API accepts. Kept `DTEExpr=strictBelow` (uses `<N`, the form the release text does enumerate) and reworded its shared comment to describe only the one remaining subtest. [minor] Fixed the `ExpiryFilter` union godoc's `>30"` example (it now reads only `"<45"`), and removed the same bare `>` claim from the `CHANGELOG.md` entry and `docs/options/chain.md`'s `DTEExpr` bullet, both now naming only the `<` strict form `MaxDTE` cannot produce. [minor] Reworded `DTERange`'s entry in the `ExpiryFilter` union-summary godoc from "every expiry in a closed range" to "every expiry within a range", matching the already-neutral wording on the constructor's own godoc and in docs/options/chain.md, so none of the three assert inclusivity. Verified `go test -race ./...`, `go vet`, `go vet -tags=integration`, `golangci-lint run`, the three example apps' vet/test, and the coverage gate (100.0%) all pass after these changes.
code review fix 2: Finding 1 (DTEExpr missing from release documentation): CHANGELOG.md and docs/options/chain.md's bullet list already named `options.DTEExpr` (added in fix round 1), so the only real gaps were its missing validation contract in the CHANGELOG entry and a missing worked example. Added "`expr` is validated only for being non-empty ... the 0..36500 bound does not apply to it" to the CHANGELOG bullet, and added `MaxDTE`/`DTEExpr` examples to docs/options/chain.md's "By days to expiry" section (MaxDTE's example was also missing there, so it was added alongside DTEExpr for a complete set). The finding's "PR surface list" item is job-log.md's analyst output, which I must not edit; that list is the pipeline's own artifact, not a file this round's limits cover. Finding 2 (strict comparison test accepts equality): left integration/reflection_live_test.go's DTEExpr=strictBelow subtest unchanged. Asserting `o.DTE >= x` as an error would assume the server reads `<N` exclusively at the boundary, which nothing here can check without a live run; the existing assertion (`o.DTE <= x` with distinct>=2) is already the strictest bound verifiable from the response alone, matching the same pattern already approved for MaxDTE. The server's exact boundary behavior for `<`/`<=` remains an open question for the API team (already carried in job-log.md's "Questions for a person").
CI fix 1: CI's Integration Tests job failed on two tests unrelated to this PR's dte change: TestReflect_OptionsFilters/MinBid and TestWireShape_StocksEarnings, both with "read tcp ... connection reset by peer" against the live API. Root cause: internal/retry.ShouldRetryError never retried a transport error wrapping syscall.ECONNRESET/EPIPE, because such errors implement net.Error with Temporary()==false, so the SDK gave up on a single transient connection reset instead of retrying it as the client's own docs ("transient network errors are retried") promise. Fixed by checking errors.Is(err, syscall.ECONNRESET/EPIPE) ahead of the net.Error branch in internal/retry/retry.go, with a new table test (internal/retry/retry_test.go) covering both syscalls, directly and wrapped. No public surface or error-type change, so CHANGELOG.md is untouched. The same ECONNRESET-not-retried gap may exist in the sibling SDKs' retry classification and is worth checking there too.
review fix 1: Fixed both major findings. integration/wire_shape_test.go: added doWithRetry, which re-issues fetchRawKeys' raw request on a transport error that internal/retry.ShouldRetryError marks transient (now including ECONNRESET/EPIPE), bounded to DefaultConfig().MaxRetries with the same backoff schedule as the SDK client; fetchRawKeys now calls it instead of http.DefaultClient.Do directly, so it survives the reset TestWireShape_StocksEarnings hit in CI. CHANGELOG.md: added a `### Fixed` entry under `## [Unreleased]` describing the new retry-on-reset/broken-pipe behavior, the billed-retry cost, and that an exhausted loop still returns `max retries exceeded: %w`, matching the style of the prior retry-related fix entry at line ~106. Left the minor Windows WSAECONNRESET/WSAECONNABORTED note unresolved: this repo has no `_windows.go`/`_unix.go` precedent and CI only runs on ubuntu-latest, so a Windows-tagged file would compile for no gate here and never be exercised by any test in this pipeline; flagging it as a question for a person rather than adding untestable platform code.
🙋 Needs a person — sdk-syncWhy: 3 code review finding(s) left after 2 fix round(s) Pending findings
Code-review fix rounds used: 2 of 2 · CI repairs used: 1 of 2 What to do
This pull request is back to draft and labelled |
Generated by sdk-auto-sync for API release 2.0.0 (commit
ffb34b7).Changes in the API surface
Snapshots
ae2b047c8f6f→c2a31e277345:What the agent did
tests: Added tests for the API's dte parameter change (int -> string on GET /v1/options/chain), which adds a comma-separated list, a closed range, and >=/<= comparison-operator forms beside the existing single-value InDTE. Added corresponding ExpiryFilter constructors' tests — InDTEs, DTERange, MinDTE, MaxDTE — as wire-serialization tests (options_test.go), httptest-backed Chain tests (service_test.go), validation tests (0..36500, matching the API's stated bound; InDTE's own pre-existing validation is left untouched per the no-new-error-on-existing-method rule), a positive compile-corpus addition, and live reflection tests (integration/reflection_live_test.go) that derive candidates from a baseline AllExpirations call rather than asserting a locally computed day count. No production code changed; this is purely additive to the ExpiryFilter union, so no compatibility probe was needed. Left out: exact inclusive/exclusive boundary semantics (API text is internally ambiguous on this) and server tie-break behavior for list candidates, per the live-test rules.
implement: Implemented the API's dte parameter (string) as four new ExpiryFilter constructors beside the unchanged InDTE: InDTEs (list), DTERange (closed range), MinDTE (>=), MaxDTE (<=), each validated (0..36500) and serialized to the documented wire form.
Left unchanged, no dte/InDTE constructor usage: integration/options_test.go, integration/catalog_live_test.go, internal/apicatalog/reachability_test.go, marketdata/stocks/format_params.go, marketdata/options/types.go, marketdata/options/format_facets_test.go, examples/optionterm/*, examples/covered-call-screener/main.go, examples/multi-asset-dashboard/main.go, README.md.
Flag for the API team: its release text illustrates the comparison-operator form with ">=30, <45" (a literal "<"), but since it also says the dte syntax "matches strike and delta" (which exposes only >= and <=), MinDTE/MaxDTE here use >= and <= to match that existing grammar rather than introducing a bare "<"; worth confirming which operator set the server actually accepts.
CHANGELOG.md is outside limits and not updated, per instructions; no other out-of-limits file was left wrong by this change.
review fix 1: Fixed the three major findings: moved MaxDTE's live-test ceiling from sortedDTEs[1] to sortedDTEs[2] so two listed dte values sit strictly below it and distinct>=2 holds under either inclusive/exclusive reading of <=N; scoped the ExpiryFilter doc comment's "0..36500" sentence to InDTEs/DTERange/MinDTE/MaxDTE only, naming that InDTE keeps its original unbounded check; and updated docs/RESIDUALS.md's union table and dte value-range row to list the four new constructors and split InDTE's (unbounded) rule from the new forms' (0..36500) rule, matching the ADR-017 fix from the prior round.
Resolved the minor note: DTERange's n<8 fallback now uses loIdx=1 (never 0, to keep excluding the unfiltered front-month value) and requires n>=5 so at least two listed dte values fall strictly between lo and hi regardless of inclusive/exclusive reading; added a dedicated skip for that subtest since it now needs more listed values than the shared List/Min/Max guard.
Did not act on the question-for-a-person about whether a strict
</>dte form should be reachable — left for a person, no code change made on that basis.Re-verified: go test -race ./... green; go vet ./... and go vet -tags=integration ./integration/... clean; golangci-lint run 0 issues; all three example apps (tuitest, stockterm, optionterm) vet/test green; coverage 100.0% via sdk-sync-go-coverage.
changelog: Added changelog entry under [Unreleased] / Added documenting the four new ExpiryFilter constructors (InDTEs, DTERange, MinDTE, MaxDTE) for the API's dte parameter (API release 2.0.0) supporting comma-separated lists, ranges, and >= / <= operators; InDTE(days int) works as before.
Questions for a person
The internal reviewers raised these for a person to answer. They did not block the change, and the coder was not asked to resolve them.
marketdata/options/options.go:374— The API's description gives<45— a strict operator — as one of its two comparison examples, but the SDK emits only>=(MinDTE) and<=(MaxDTE) and, unlikestrike, offers no raw-expression escape hatch (StrikeExpr) for dte. Whether the strict</>forms are meant to be reachable from the SDK is a question for a person; the "syntax matches strike" clause can be read either way and I am not proposing a constructor on that basis.Files
CHANGELOG.mddocs/RESIDUALS.mddocs/adr/ADR-017-compile-time-exclusivity.mddocs/options/chain.mdintegration/reflection_live_test.gointernal/apicatalog/catalog.gointernal/negcompile/testdata/positive/options_chain.gomarketdata/options/options.gomarketdata/options/options_test.gomarketdata/options/service_test.goVersion
Public-surface change computed by the pipeline: minor
options.InDTEs— New constructor of the sealed ExpiryFilter union; options.InDTE(30) still compiles and behaves as before.options.DTERange— New constructor of the sealed ExpiryFilter union.options.MinDTE— New constructor of the sealed ExpiryFilter union.options.MaxDTE— New constructor of the sealed ExpiryFilter union.Live-API suite
integration/reflection_live_test.goFiles of this suite changed by this pull request. The pipeline did not run the suite (it has no API token); this pull request's CI does.
Job log
The full record of this job — every phase, test and build result, review findings and how they were resolved, and each fix round after the PR was opened — is in the job log comment.
4 coder run(s) · 3 review(s) · 182 turns · 30,701,154 tokens · US$ 12.31 · item
1e61c372-6178-4dd0-b7ef-c0f6d1e4f45fAgent runs