Skip to content

fix(graphql): add time range support and point cap validation to getPriceHistory (#370) - #390

Merged
Miracle656 merged 2 commits into
Divinity-Labs:mainfrom
Ahmad12developer:fix/issue-370-get-price-history-time-range
Oct 18, 2026
Merged

Miracle656 merged 2 commits into
Divinity-Labs:mainfrom
Ahmad12developer:fix/issue-370-get-price-history-time-range

Conversation

@Ahmad12developer

Copy link
Copy Markdown
Contributor

Summary

Implements optional from and to ISO 8601 time range filtering for the getPriceHistory GraphQL query, documents boundary inclusivity in the schema description, and enforces point cap bounds matching the REST route.

Fixes

Fixes #370

Details of Changes

  1. Schema & Documentation:
    • Added optional from: String and to: String arguments to getPriceHistory in the GraphQL schema.
    • Documented boundary inclusivity in the schema description: the range is inclusive at both ends (bucket >= from AND bucket <= to), noting that adjacent query windows intentionally share boundary buckets.
  2. Input & Bound Validation:
    • Validated from and to as ISO 8601 timestamps, rejecting invalid date strings with descriptive error messages.
    • Enforced that from must be before or equal to to, rejecting inverted ranges (from must be before to).
    • Enforced MAX_HISTORY_POINTS (10,000) cap matching the REST /prices/history route to prevent unbounded DB scans over large time spans at fine resolutions.
  3. Database Querying:
    • Dynamically parameterized the PostgreSQL query in src/api/graphql.ts to add bucket >= and bucket <= conditions when provided.
    • Preserved default descending order and capped limit at 1000.
    • Maintained full backward compatibility for queries without from or to.
  4. Testing:
    • Added comprehensive integration test suite in src/__tests__/graphqlPriceHistory.test.ts verifying:
      • Schema description explicitly documents boundary inclusivity and arguments.
      • Rejection of invalid from and to strings.
      • Rejection when from is after to.
      • Rejection of over-large windows exceeding MAX_HISTORY_POINTS.
      • Successful querying and parameter mapping for valid bounded ranges.
      • Single-bound and legacy queries without from/to.
    • Verified all 647 unit/integration tests pass with zero regressions.

Checklist

  • from and to are supported and validated
  • An over-large range is rejected rather than served
  • from after to is an error
  • Boundary inclusivity is documented in the schema description
  • Tests cover a bounded range, an invalid range and an over-large one
  • All 647 test cases pass cleanly (npm test)

…riceHistory (Divinity-Labs#370)

- Add optional from and to ISO 8601 string arguments to getPriceHistory query.
- Document boundary inclusivity in schema description (bucket >= from AND bucket <= to).
- Validate from and to as valid ISO 8601 dates and enforce from before to.
- Enforce MAX_HISTORY_POINTS bound matching the REST route to reject over-large ranges.
- Dynamically add parameterized bucket >= and bucket <= conditions to PostgreSQL query.
- Add comprehensive test suite in src/__tests__/graphqlPriceHistory.test.ts covering valid bounded ranges, invalid dates, inverted ranges, over-large windows, and legacy calls.

Closes Divinity-Labs#370
@Miracle656

Copy link
Copy Markdown
Collaborator

Needs a rebase — main moved six times this morning (#379, #380, #382, #392, #401, #310 all landed) and this now conflicts. Push a rebase and I will pick it up; nothing is queued ahead of it.

@Ahmad12developer

Copy link
Copy Markdown
Contributor Author

Resolved merge conflicts with latest upstream/main and integrated with GraphQL error masking. All 673 tests passing and build succeeds. @coderabbitai review

@Miracle656 Miracle656 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Every criterion met, and one of them met in the way I actually wanted rather than the way that was easiest.

Boundary inclusivity is stated, not assumed. The schema description says it outright and names the consequence:

The time range is inclusive at both ends… Because both endpoints are inclusive, querying adjacent windows (e.g. [t0, t1] and [t1, t2]) will…

That was the point of the criterion. Range queries in this codebase are inclusive at both ends, so adjacent windows double-count the boundary — and the failure mode is that nobody notices until a sum is quietly wrong. Writing it into the schema puts it in front of the one person who can act on it, and documents boundary inclusivity in the schema description is a test that keeps it there.

The cap is imported, not reimplemented. HISTORY_INTERVAL_SECONDS and MAX_HISTORY_POINTS come from ./history, so GraphQL and REST cannot drift into disagreeing about how large a window is too large. Copying the constant is the obvious shortcut and it is how two surfaces end up with different limits that nobody can explain a year later.

The test set covers the right shapes too — invalid from, invalid to, from after to, an over-large range, a bounded range asserting the query really is inclusive, only-from, and legacy calls with neither argument. That last one is what says existing clients still work.

Merging.

@Miracle656
Miracle656 merged commit 75d0556 into Divinity-Labs:main Oct 18, 2026
1 check passed
Miracle656 added a commit to Bigclavsfx/Lens that referenced this pull request Oct 18, 2026
The drift guard this PR adds caught its own artifact going stale:

    × committed schema.graphql matches the code-defined schema exactly

Between this branch being cut and now, Divinity-Labs#380 (invalid/unknown/empty pair
distinctions), Divinity-Labs#381 (the oracle query and OracleSourceStatus enum) and
Divinity-Labs#390 (from/to on getPriceHistory) all changed the code-defined schema.

    npm run graphql:schema:gen

Which is the guard doing exactly what it was written for, on its first
outing.

graphqlSchemaDrift.test.ts: 6 passed.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
Miracle656 added a commit that referenced this pull request Oct 18, 2026
#476)

* feat(graphql): export schema SDL artifact, add drift guard and breaking change check

* chore(graphql): regenerate schema.graphql after the schema moved on main

The drift guard this PR adds caught its own artifact going stale:

    × committed schema.graphql matches the code-defined schema exactly

Between this branch being cut and now, #380 (invalid/unknown/empty pair
distinctions), #381 (the oracle query and OracleSourceStatus enum) and
#390 (from/to on getPriceHistory) all changed the code-defined schema.

    npm run graphql:schema:gen

Which is the guard doing exactly what it was written for, on its first
outing.

graphqlSchemaDrift.test.ts: 6 passed.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

---------

Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>
This was referenced Oct 18, 2026
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.

getPriceHistory cannot express a time range

2 participants