Skip to content

feat(api): meter query v3#3918

Merged
tothandras merged 1 commit intomainfrom
feat/api-v3-query-meter
Mar 5, 2026
Merged

feat(api): meter query v3#3918
tothandras merged 1 commit intomainfrom
feat/api-v3-query-meter

Conversation

@tothandras
Copy link
Contributor

@tothandras tothandras commented Mar 3, 2026

Summary by CodeRabbit

  • New Features

    • Added a meter query API (POST /openmeter/meters/{meterId}) with granularity-based bucketing, time-zone defaulting, filtering, grouping, and windowed results; new request/response shapes and examples.
  • Refactor

    • Renamed and reorganized filter models to QueryFilter* with normalized types, operators, composition semantics, and string-map support.
  • Bug Fixes

    • Ignored specific prefixes in prefix-grouping logic and removed related suppression directives.
  • Chores

    • Added a dev dependency to the spec tooling.

@tothandras tothandras requested a review from a team as a code owner March 3, 2026 22:47
@tothandras tothandras added the release-note/feature Release note: Exciting New Features label Mar 3, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 3, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a MeterQuery feature (new query models + POST /meters/{meterId} endpoint), refactors shared filter models from Metering* → QueryFilter*, updates a field-prefix lint rule to exclude specific prefixes, and removes related suppression directives in two spec files. Generated Go server types and a route handler stub were added.

Changes

Cohort / File(s) Summary
Rule & Linting
api/spec/lib/rules/field-prefix.js
Added EXCLUDED_PREFIXES and a guard to skip reporting prefixes in that list.
Suppression Removal
api/spec/src/v3/apps/external_invoicing.tsp, api/spec/src/v3/customers/billing.tsp
Removed @openmeter/api-spec/repeated-prefix-grouping suppression directives.
Meter Query API Spec
api/spec/src/v3/meters/query.tsp, api/spec/src/v3/meters/operations.tsp
New MeterQuery models (MeterQueryGranularity, MeterQueryFilters, MeterQueryRequest, MeterQueryResult, MeterQueryRow) and a POST /meters/{meterId} operation added to MetersOperations.
Filter Model Refactor
api/spec/src/v3/shared/filters.tsp
Renamed Metering* filters to QueryFilter*, normalized operator names/types, added map-item filters and logical combinators, adjusted numeric/date types.
Generated Server & Types
api/v3/api.gen.go, api/v3/server/routes.go
Generated Go types/aliases for MeterQuery, added QueryMeter to server interface/wrapper, routing registration (POST /openmeter/meters/{meterId}), and server handler stub.
Spec Tooling
api/spec/package.json
Added devDependency @typespec/json-schema (dev tooling change).

Sequence Diagram

sequenceDiagram
    participant Client as Client
    participant API as "Router / API Gateway"
    participant Handler as "Server Handler (QueryMeter)"
    participant Store as "Meter Store / DB"

    Client->>API: POST /openmeter/meters/{meterId} (MeterQueryRequest)
    API->>Handler: invoke QueryMeter(meterId, request)
    Handler->>Store: execute query(filters, time range, group_by, granularity)
    Store->>Store: aggregate/window data
    Store-->>Handler: return MeterQueryRow[]
    Handler-->>API: return MeterQueryResult
    API-->>Client: 200 OK + MeterQueryResult
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested labels

area/api

Suggested reviewers

  • turip
  • hekike
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(api): meter query v3' clearly and concisely summarizes the main change—adding a v3 meter query feature to the API. It's specific, follows conventional commit format, and accurately reflects the substantial new functionality across multiple files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/api-v3-query-meter

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
api/spec/src/v3/meters/query.tsp (1)

156-158: Consider tightening customer_id to Shared.ULID.

Using plain string here drops identifier-format validation for a field that appears to be a customer ID.

💡 Suggested spec tweak
-  customer_id?: string;
+  customer_id?: Shared.ULID;

As per coding guidelines: "The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/spec/src/v3/meters/query.tsp` around lines 156 - 158, The customer_id
field is declared as a plain string which bypasses ULID validation; update the
type annotation for customer_id from string to Shared.ULID in the meters query
spec so the schema enforces the ULID format (locate the customer_id property in
the query type/interface definition in api/spec/src/v3/meters/query.tsp and
change its type to Shared.ULID).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/v3/server/routes.go`:
- Around line 29-31: The handler QueryMeter currently delegates to
unimplemented.QueryMeter and must be wired to the server's actual meter logic:
replace the call to unimplemented.QueryMeter in func (s *Server) QueryMeter(w
http.ResponseWriter, r *http.Request, meterId api.ULID) with code that uses the
Server's meter service/store (e.g., s.MeterStore, s.MeterService, or whatever
central type on Server holds meter logic) to perform the query using the
provided meterId, handle request body/params if the endpoint expects a POST
payload, marshal the result to JSON and write it to w with the appropriate
status (200 on success), and return proper error responses and status codes on
not-found or failure (including logging). Ensure you reference and call the
concrete method on the server's meter component (e.g., MeterService.QueryMeter
or MeterStore.GetMeter) and not unimplemented.QueryMeter so the POST
/openmeter/meters/{meterId} endpoint becomes functional.

---

Nitpick comments:
In `@api/spec/src/v3/meters/query.tsp`:
- Around line 156-158: The customer_id field is declared as a plain string which
bypasses ULID validation; update the type annotation for customer_id from string
to Shared.ULID in the meters query spec so the schema enforces the ULID format
(locate the customer_id property in the query type/interface definition in
api/spec/src/v3/meters/query.tsp and change its type to Shared.ULID).

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57478c6 and ae64bf7.

⛔ Files ignored due to path filters (1)
  • api/v3/openapi.yaml is excluded by !**/openapi.yaml
📒 Files selected for processing (8)
  • api/spec/lib/rules/field-prefix.js
  • api/spec/src/v3/apps/external_invoicing.tsp
  • api/spec/src/v3/customers/billing.tsp
  • api/spec/src/v3/meters/operations.tsp
  • api/spec/src/v3/meters/query.tsp
  • api/spec/src/v3/shared/filters.tsp
  • api/v3/api.gen.go
  • api/v3/server/routes.go
💤 Files with no reviewable changes (2)
  • api/spec/src/v3/customers/billing.tsp
  • api/spec/src/v3/apps/external_invoicing.tsp

@tothandras tothandras force-pushed the feat/api-v3-query-meter branch from ae64bf7 to 850bdcf Compare March 4, 2026 11:48
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
api/spec/src/v3/shared/filters.tsp (1)

127-166: Consolidate QueryFilterFloat and QueryFilterNumeric to use Shared.Numeric

QueryFilterFloat and QueryFilterNumeric have identical structure but different types—float64 vs. Shared.Numeric. Since Shared.Numeric is the project standard for arbitrary-precision numeric values (and Money is aliased to it), using float64 risks precision drift and creates unnecessary API surface confusion. Either consolidate to a single filter using Shared.Numeric, or deprecate QueryFilterFloat.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/spec/src/v3/shared/filters.tsp` around lines 127 - 166, QueryFilterFloat
currently uses float64 and duplicates QueryFilterNumeric; replace all float64
usages in the QueryFilterFloat model (gt, gte, lt, lte) with Shared.Numeric and
keep the and?: QueryFilterFloat[] and or?: QueryFilterFloat[] so the model
aligns with the project-wide numeric type (Shared.Numeric) and avoids precision
drift/API duplication; alternatively, remove QueryFilterFloat and update callers
to use QueryFilterNumeric if you prefer full consolidation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@api/spec/src/v3/shared/filters.tsp`:
- Around line 127-166: QueryFilterFloat currently uses float64 and duplicates
QueryFilterNumeric; replace all float64 usages in the QueryFilterFloat model
(gt, gte, lt, lte) with Shared.Numeric and keep the and?: QueryFilterFloat[] and
or?: QueryFilterFloat[] so the model aligns with the project-wide numeric type
(Shared.Numeric) and avoids precision drift/API duplication; alternatively,
remove QueryFilterFloat and update callers to use QueryFilterNumeric if you
prefer full consolidation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3d493f35-d5bc-4fb0-8d66-c2d4d7e28199

📥 Commits

Reviewing files that changed from the base of the PR and between ae64bf7 and 850bdcf.

⛔ Files ignored due to path filters (1)
  • api/v3/openapi.yaml is excluded by !**/openapi.yaml
📒 Files selected for processing (8)
  • api/spec/lib/rules/field-prefix.js
  • api/spec/src/v3/apps/external_invoicing.tsp
  • api/spec/src/v3/customers/billing.tsp
  • api/spec/src/v3/meters/operations.tsp
  • api/spec/src/v3/meters/query.tsp
  • api/spec/src/v3/shared/filters.tsp
  • api/v3/api.gen.go
  • api/v3/server/routes.go
💤 Files with no reviewable changes (2)
  • api/spec/src/v3/apps/external_invoicing.tsp
  • api/spec/src/v3/customers/billing.tsp
🚧 Files skipped from review as they are similar to previous changes (4)
  • api/v3/server/routes.go
  • api/spec/src/v3/meters/query.tsp
  • api/spec/lib/rules/field-prefix.js
  • api/spec/src/v3/meters/operations.tsp

@tothandras tothandras force-pushed the feat/api-v3-query-meter branch from 850bdcf to 9c911ff Compare March 4, 2026 12:30
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
api/spec/src/v3/shared/filters.tsp (1)

153-173: Prefer Shared.Numeric over float64 in QueryFilterFloat.

Using binary floats in API filter inputs can introduce precision drift across clients/backends. Switching to Shared.Numeric keeps behavior aligned with the rest of the precision-safe numeric surface.

💡 Suggested change
 model QueryFilterFloat {
   /**
    * The attribute is greater than the provided value.
    */
-  gt?: float64;
+  gt?: Shared.Numeric;

   /**
    * The attribute is greater than or equal to the provided value.
    */
-  gte?: float64;
+  gte?: Shared.Numeric;

   /**
    * The attribute is less than the provided value.
    */
-  lt?: float64;
+  lt?: Shared.Numeric;

   /**
    * The attribute is less than or equal to the provided value.
    */
-  lte?: float64;
+  lte?: Shared.Numeric;

Based on learnings: "prefer using Shared.Numeric ... broadly to all TypeSpec files under api/spec to ensure consistent precision handling and serialization."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/spec/src/v3/shared/filters.tsp` around lines 153 - 173, The
QueryFilterFloat model uses float64 for gt, gte, lt, and lte which can cause
precision drift; update the type of those properties to Shared.Numeric instead
to match the precision-safe numeric surface used elsewhere. Locate the
QueryFilterFloat model and replace each float64 occurrence for gt, gte, lt, and
lte with Shared.Numeric, and ensure any necessary references/imports to
Shared.Numeric are present so the model compiles and serializes consistently
with other API specs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/spec/src/v3/meters/query.tsp`:
- Line 23: The filters.dimensions property (currently typed as dimensions?:
Shared.QueryFilterStringMap) is unbounded and should be capped like
group_by_dimensions to avoid oversized payloads; update the type of dimensions
(or the surrounding interface) to enforce a maximum number of keys (the same
hard cap used by group_by_dimensions) and validate/reflect this constraint in
the type definition and any runtime validation code that constructs or accepts
the query (reference the symbols dimensions, filters.dimensions,
Shared.QueryFilterStringMap and group_by_dimensions when making the change).

In `@api/spec/src/v3/shared/filters.tsp`:
- Around line 67-79: QueryFilterStringMapItem currently spreads
...QueryFilterString which defines and/or as QueryFilterString[], causing nested
clauses to lose the map-item-specific flags (exists/nexists); modify the schema
so the recursive branches reference the map-item shape instead of
QueryFilterString — e.g., make and and or on QueryFilterStringMapItem typed as
QueryFilterStringMapItem[] (or replace the spread with an explicit
self-recursive definition) so exists and nexists remain available inside nested
and/or; update the definition of QueryFilterStringMapItem (and remove or adjust
the ...QueryFilterString spread) to enforce this recursive map-item type.

---

Nitpick comments:
In `@api/spec/src/v3/shared/filters.tsp`:
- Around line 153-173: The QueryFilterFloat model uses float64 for gt, gte, lt,
and lte which can cause precision drift; update the type of those properties to
Shared.Numeric instead to match the precision-safe numeric surface used
elsewhere. Locate the QueryFilterFloat model and replace each float64 occurrence
for gt, gte, lt, and lte with Shared.Numeric, and ensure any necessary
references/imports to Shared.Numeric are present so the model compiles and
serializes consistently with other API specs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f3912450-fbb9-4fff-8a7e-0e0165e81a1b

📥 Commits

Reviewing files that changed from the base of the PR and between 850bdcf and 9c911ff.

⛔ Files ignored due to path filters (1)
  • api/v3/openapi.yaml is excluded by !**/openapi.yaml
📒 Files selected for processing (8)
  • api/spec/lib/rules/field-prefix.js
  • api/spec/src/v3/apps/external_invoicing.tsp
  • api/spec/src/v3/customers/billing.tsp
  • api/spec/src/v3/meters/operations.tsp
  • api/spec/src/v3/meters/query.tsp
  • api/spec/src/v3/shared/filters.tsp
  • api/v3/api.gen.go
  • api/v3/server/routes.go
💤 Files with no reviewable changes (2)
  • api/spec/src/v3/customers/billing.tsp
  • api/spec/src/v3/apps/external_invoicing.tsp
🚧 Files skipped from review as they are similar to previous changes (3)
  • api/v3/server/routes.go
  • api/spec/src/v3/meters/operations.tsp
  • api/spec/lib/rules/field-prefix.js

@tothandras tothandras force-pushed the feat/api-v3-query-meter branch from 9c911ff to e40e6a7 Compare March 4, 2026 23:31
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
api/spec/src/v3/meters/query.tsp (1)

23-23: ⚠️ Potential issue | 🟠 Major

Please cap filters.dimensions size.

Line 23 is currently unbounded. A large dimensions object can create expensive query plans and oversized payloads.

💡 Suggested change
 model MeterQueryFilters {
   /**
    * Filters to apply to the dimensions of the query.
    */
+  `@maxProperties`(100)
   dimensions?: Shared.QueryFilterStringMap;
 }

As per coding guidelines: "The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/spec/src/v3/meters/query.tsp` at line 23, The dimensions property
currently typed as Shared.QueryFilterStringMap is unbounded; add a bounded
variant (e.g., Shared.QueryFilterStringMapLimited or DimensionsLimited) and
change the declaration from dimensions?: Shared.QueryFilterStringMap to
dimensions?: Shared.QueryFilterStringMapLimited with a documented max entries
limit (suggest 50). Also add runtime validation where the meters query payload
is processed (the request validator / handler that reads filters.dimensions) to
reject requests exceeding the max with a clear error. Update any related
schema/typing helpers and tests to use the new Limited type and validation rule
so the API surface and implementation stay in parity.
api/spec/src/v3/shared/filters.tsp (1)

67-79: ⚠️ Potential issue | 🟠 Major

QueryFilterStringMapItem loses map-item semantics in nested and/or.

Because Line 78 spreads QueryFilterString, nested and/or are typed as QueryFilterString[], so nested clauses cannot carry exists/nexists.

💡 Suggested shape (self-recursive map-item branches)
 model QueryFilterStringMapItem {
   /**
    * The attribute exists.
    */
   exists?: boolean;

   /**
    * The attribute does not exist.
    */
   nexists?: boolean;

-  ...QueryFilterString;
+  eq?: string;
+  neq?: string;
+  `@minItems`(1)
+  `@maxItems`(100)
+  in?: string[];
+  `@minItems`(1)
+  `@maxItems`(100)
+  nin?: string[];
+  contains?: string;
+  ncontains?: string;
+
+  `@minItems`(1)
+  `@maxItems`(10)
+  and?: QueryFilterStringMapItem[];
+
+  `@minItems`(1)
+  `@maxItems`(10)
+  or?: QueryFilterStringMapItem[];
 }
#!/bin/bash
# Verify generated recursion shape from the spec.
rg -n -C2 'type QueryFilterStringMapItem struct|And \*\[\]QueryFilterString|Or \*\[\]QueryFilterString' api/v3/api.gen.go

As per coding guidelines: "The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/spec/src/v3/shared/filters.tsp` around lines 67 - 79,
QueryFilterStringMapItem currently spreads QueryFilterString which makes nested
and/or resolve to QueryFilterString[] and lose the map-item fields
(exists/nexists); change the schema so QueryFilterStringMapItem retains its own
fields and makes the logical branches self-recursive (i.e., define and and or on
QueryFilterStringMapItem as arrays of QueryFilterStringMapItem or otherwise
replace the spread of QueryFilterString with an explicit merge that sets and?:
QueryFilterStringMapItem[] and or?: QueryFilterStringMapItem[] while preserving
all QueryFilterString properties, ensuring nested clauses keep exists/nexists;
update any related type aliases that reference QueryFilterString to accept this
recursive map-item shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/spec/src/v3/shared/filters.tsp`:
- Line 21: The generated API types are out of sync with the TypeSpec changes:
the spec now defines filters containing the fields `contains` and `ncontains` in
filters.tsp but the generated types still expose the legacy `Like` and `Nlike`
fields; update the code generation or mapping so the generated models use
`contains` and `ncontains` instead of `Like`/`Nlike`. Locate the filter type
declarations (e.g., the filter interface/type that currently includes `Like` and
`Nlike`) and modify the generator/templates or transformation step to emit
`contains?: string` and `ncontains?: string` (and remove `Like`/`Nlike`), then
re-run the TypeSpec generation to ensure the API types match the spec (also
apply the same fix to the other affected declarations referenced around the
block that includes lines 40–45).

In `@api/v3/api.gen.go`:
- Around line 3938-3942: Add a short explanatory comment above the
Unimplemented.QueryMeter handler stating that the POST
/openmeter/meters/{meterId} endpoint is intentionally left as a 501 stub while
the v3 feature is scaffolded and that routes delegate to
unimplemented.QueryMeter until a full implementation is added; reference the
Unimplemented.QueryMeter function name and note that this is deliberate to avoid
the handler being mistaken for an accidental omission.

---

Duplicate comments:
In `@api/spec/src/v3/meters/query.tsp`:
- Line 23: The dimensions property currently typed as
Shared.QueryFilterStringMap is unbounded; add a bounded variant (e.g.,
Shared.QueryFilterStringMapLimited or DimensionsLimited) and change the
declaration from dimensions?: Shared.QueryFilterStringMap to dimensions?:
Shared.QueryFilterStringMapLimited with a documented max entries limit (suggest
50). Also add runtime validation where the meters query payload is processed
(the request validator / handler that reads filters.dimensions) to reject
requests exceeding the max with a clear error. Update any related schema/typing
helpers and tests to use the new Limited type and validation rule so the API
surface and implementation stay in parity.

In `@api/spec/src/v3/shared/filters.tsp`:
- Around line 67-79: QueryFilterStringMapItem currently spreads
QueryFilterString which makes nested and/or resolve to QueryFilterString[] and
lose the map-item fields (exists/nexists); change the schema so
QueryFilterStringMapItem retains its own fields and makes the logical branches
self-recursive (i.e., define and and or on QueryFilterStringMapItem as arrays of
QueryFilterStringMapItem or otherwise replace the spread of QueryFilterString
with an explicit merge that sets and?: QueryFilterStringMapItem[] and or?:
QueryFilterStringMapItem[] while preserving all QueryFilterString properties,
ensuring nested clauses keep exists/nexists; update any related type aliases
that reference QueryFilterString to accept this recursive map-item shape.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 17add27c-5bd5-4248-bd1b-207a2adb6b5e

📥 Commits

Reviewing files that changed from the base of the PR and between 9c911ff and e40e6a7.

⛔ Files ignored due to path filters (1)
  • api/v3/openapi.yaml is excluded by !**/openapi.yaml
📒 Files selected for processing (8)
  • api/spec/lib/rules/field-prefix.js
  • api/spec/src/v3/apps/external_invoicing.tsp
  • api/spec/src/v3/customers/billing.tsp
  • api/spec/src/v3/meters/operations.tsp
  • api/spec/src/v3/meters/query.tsp
  • api/spec/src/v3/shared/filters.tsp
  • api/v3/api.gen.go
  • api/v3/server/routes.go
💤 Files with no reviewable changes (2)
  • api/spec/src/v3/customers/billing.tsp
  • api/spec/src/v3/apps/external_invoicing.tsp
🚧 Files skipped from review as they are similar to previous changes (1)
  • api/v3/server/routes.go

Comment on lines +3938 to +3942
// Query meter
// (POST /openmeter/meters/{meterId})
func (_ Unimplemented) QueryMeter(w http.ResponseWriter, r *http.Request, meterId ULID) {
w.WriteHeader(http.StatusNotImplemented)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Search for all QueryMeter function implementations
find . -type f -name "*.go" | head -20
rg -n --type=go "QueryMeter" | head -30

Repository: openmeterio/openmeter

Length of output: 4232


🏁 Script executed:

# Look for the ServerInterface and concrete implementations
rg -n --type=go "type.*ServerInterface" -A 5 | head -50

Repository: openmeterio/openmeter

Length of output: 1370


🏁 Script executed:

# Check for route registration and handler setup
rg -n --type=go "openmeter/meters" | head -20

Repository: openmeterio/openmeter

Length of output: 1114


🏁 Script executed:

# Look for the unimplemented pattern and how it's handled
rg -n --type=go "Unimplemented" -A 2 | head -40

Repository: openmeterio/openmeter

Length of output: 1918


🏁 Script executed:

# Look at the Router struct and see what methods it implements
rg -n --type=go "type Router struct" -A 20 openmeter/server/router/

# Look at the meter.go file to see QueryMeter implementations
cat -n openmeter/server/router/meter.go | head -75

Repository: openmeterio/openmeter

Length of output: 5066


🏁 Script executed:

# Check if Router implements the ServerInterface by looking for all its methods
rg -n --type=go "^func \(a \*Router\)" openmeter/server/router/meter.go

Repository: openmeterio/openmeter

Length of output: 1047


🏁 Script executed:

# See the actual wrapper delegation pattern
sed -n '4041,4100p' api/v3/api.gen.go

Repository: openmeterio/openmeter

Length of output: 1987


🏁 Script executed:

# Check the v3/server/routes.go file
cat -n api/v3/server/routes.go | head -50

Repository: openmeterio/openmeter

Length of output: 1886


🏁 Script executed:

# Look for ServerInterface implementations
rg -n --type=go "func \(.*\) QueryMeter\(" | grep -v "streaming"

Repository: openmeterio/openmeter

Length of output: 1092


🏁 Script executed:

# Check if Router is used for v3
rg -n --type=go "api\.NewServerInterfaceToHandlerWithOptions|api\.Unimplemented" api/v3/

Repository: openmeterio/openmeter

Length of output: 130


🏁 Script executed:

# Check if there are any TODOs or comments around the QueryMeter implementation
sed -n '25,35p' api/v3/server/routes.go

Repository: openmeterio/openmeter

Length of output: 440


🏁 Script executed:

# Check git to see when this was added or if there are related comments
git log --oneline -20 -- api/v3/server/routes.go 2>/dev/null || echo "Git history not available"

Repository: openmeterio/openmeter

Length of output: 99


QueryMeter (POST /openmeter/meters/{meterId}) is intentionally stubbed to return 501.

The endpoint is wired and routed, but api/v3/server/routes.go:29–31 explicitly delegates to unimplemented.QueryMeter(). Since this is scaffolding for the v3 feature, consider adding a brief comment documenting that the handler implementation is pending—just so future devs (and your future self) know this is intentional and not forgotten.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/v3/api.gen.go` around lines 3938 - 3942, Add a short explanatory comment
above the Unimplemented.QueryMeter handler stating that the POST
/openmeter/meters/{meterId} endpoint is intentionally left as a 501 stub while
the v3 feature is scaffolded and that routes delegate to
unimplemented.QueryMeter until a full implementation is added; reference the
Unimplemented.QueryMeter function name and note that this is deliberate to avoid
the handler being mistaken for an accidental omission.

@tothandras tothandras force-pushed the feat/api-v3-query-meter branch from e40e6a7 to bfe972b Compare March 5, 2026 15:46
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/spec/src/v3/meters/query.tsp`:
- Around line 20-21: The Hour granularity literal is using a non-standard
ISO-8601 token "P1H"; update the literal for the Hour enum/constant (symbol:
Hour) to the correct ISO-8601 duration "PT1H" so parsers/clients receive the
proper one-hour duration value and maintain compatibility.
- Around line 42-51: The JSDoc for subject_key and customer_id claims only
"eq"/"in" are supported, but their type Shared.QueryFilterString includes
additional operators; fix by aligning the schema and docs: either narrow the
property types (e.g., introduce and use a Dedicated type that only allows eq/in,
such as QueryFilterStringEqIn) for subject_key and customer_id, or expand the
doc to enumerate all supported operators and behaviors exposed by
Shared.QueryFilterString; update references to Shared.QueryFilterString,
subject_key, and customer_id so the declared type and the comment are
consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dcf394c3-1eec-4b0d-9b80-8516f5355854

📥 Commits

Reviewing files that changed from the base of the PR and between e40e6a7 and bfe972b.

⛔ Files ignored due to path filters (2)
  • api/spec/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • api/v3/openapi.yaml is excluded by !**/openapi.yaml
📒 Files selected for processing (9)
  • api/spec/lib/rules/field-prefix.js
  • api/spec/package.json
  • api/spec/src/v3/apps/external_invoicing.tsp
  • api/spec/src/v3/customers/billing.tsp
  • api/spec/src/v3/meters/operations.tsp
  • api/spec/src/v3/meters/query.tsp
  • api/spec/src/v3/shared/filters.tsp
  • api/v3/api.gen.go
  • api/v3/server/routes.go
💤 Files with no reviewable changes (2)
  • api/spec/src/v3/customers/billing.tsp
  • api/spec/src/v3/apps/external_invoicing.tsp
🚧 Files skipped from review as they are similar to previous changes (4)
  • api/spec/src/v3/meters/operations.tsp
  • api/spec/src/v3/shared/filters.tsp
  • api/spec/lib/rules/field-prefix.js
  • api/v3/server/routes.go

@tothandras tothandras force-pushed the feat/api-v3-query-meter branch from f4a0f9f to 28b265d Compare March 5, 2026 18:39
@tothandras tothandras merged commit cd53796 into main Mar 5, 2026
24 checks passed
@tothandras tothandras deleted the feat/api-v3-query-meter branch March 5, 2026 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/feature Release note: Exciting New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants