Skip to content

feat(api): document measured-power contract and add powerValid filter | API:文档化实测功率契约并新增 powerValid 筛选参数 - #938

Open
edwingao28 wants to merge 6 commits into
feat/etl-invalid-power-scrubfrom
feat/api-power-contract
Open

feat(api): document measured-power contract and add powerValid filter | API:文档化实测功率契约并新增 powerValid 筛选参数#938
edwingao28 wants to merge 6 commits into
feat/etl-invalid-power-scrubfrom
feat/api-power-contract

Conversation

@edwingao28

@edwingao28 edwingao28 commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Important

STACKED PR — base is feat/etl-invalid-power-scrub (#937). That PR must merge first. This PR contains only the deltas on top of #937; do not merge it into master before #937 lands, and retarget to master after #937 merges.

What | 内容

Adds a typed, documented public-API measured-power contract plus a powerValid row filter on GET /api/v1/benchmarks. | 为公开 API 提供带类型、有文档的实测功率契约,并为 GET /api/v1/benchmarks 新增 powerValid 行筛选参数。

  1. POWER_METRIC_KEYS single source of truth (packages/constants/src/metric-keys.ts): the two contract discriminators (power_valid, power_metric_schema_version) plus MEASURED_POWER_METRIC_KEY_LIST (from fix(etl): strip measured power metrics at ingest when power_valid=0 | ETL:power_valid=0 时在摄取阶段剥离实测功耗指标 #937), spread into METRIC_KEYS so set membership is provably unchanged (test-asserted).
  2. Documentation (api-documentation.ts):
  3. Filter (src/lib/benchmark-power-validity.ts, pure post-cache): powerValid ∈ {1, 0, any, strictV2}, default any (identity — byte-identical to today). strictV2power_valid == 1 && power_metric_schema_version == 2. Unknown values → 400 Unknown powerValid filter; combining with view=calculator → 400 (that cache stores rows already trimmed past power_valid). Cache keys unchanged — no blob-cache fragmentation.
  4. Ledger: stablePublicApiContracts.list-benchmarks.parameters extended in order; route digest recomputed.

Design decision: strictV2 naming (UI-vs-API divergence) | 设计决策:strictV2 命名(界面与 API 的差异)

The strict filter value is named strictV2, not certified: the product UI's "certified" tier is a display rule that also admits validated legacy rows without a schema version, which this filter excludes. Using certified in the API would give the same word two different meanings across surfaces. The divergence is documented in the measured-power schema note and the powerValid param description. certified is explicitly rejected by the parser (test-pinned) so it can never silently alias. | 严格筛选值命名为 strictV2 而非 certified:产品界面的“认证”档位还会展示无 schema 版本的已验证旧行,而此筛选会排除它们。若 API 也用 certified,同一词汇将在两个界面承载不同含义。该差异已写入 measured-power 说明与参数描述。

Backward compatibility | 向后兼容

  • Absent powerValidpowerValid=any ≡ today's behavior (explicit route test asserts identical output; all 17 pre-existing route tests unchanged and green).
  • All new parameters optional; no cache-key changes; no scrubbing of API responses (raw-row contract unchanged); reserved OpenAPI fields are documentation-only.
  • METRIC_KEYS membership unchanged.

Tests | 测试

  • packages/constants bun run test:unit: 53 passed (incl. new POWER_METRIC_KEYS assertions in metric-keys.test.ts).
  • packages/app bun run test:unit: 252 files / 4476 passed, including:
    • api-route-catalog.test.ts (digest + parameter/status/error parity, bilingual completeness, OpenAPI projection parity)
    • route.test.ts (17 existing + 8 new filter/400/compat cases)
    • new benchmark-power-validity.test.ts, api-documentation.power.test.ts
  • packages/db bun run test:unit: 609 passed (no coupling).
  • Cypress api-documentation.cy.ts (fixtures mode): 1/1 passed — /api, /zh/api, and openapi.json expose the new contract.
  • Manual smoke on the fixtures server: 400s verified; absent vs any responses byte-identical.

Review notes | 评审说明

  1. The published copy said powerValid "cannot be combined with view=calculator" without the any exception, while the route accepts powerValid=any&view=calculator (200, no-op filter). Resolved in 0e691a4: the operation description, the powerValid param description, and the 400 response description now state the exception (EN + ZH). The docs are no longer stricter than actual behavior. | 文档此前称 powerValid 不能与 view=calculator 组合,但未提及 any 例外(路由接受 powerValid=any&view=calculator 并返回 200)。已在 0e691a4 修复:操作描述、参数描述与 400 响应描述均已注明该例外(中英双语)。

No other findings.

Rollback | 回滚

Revert this PR. No migrations, no cache-key changes, no data writes.


Note

Medium Risk
Expands the stable public benchmarks contract and response filtering semantics; default behavior stays unchanged, but integrators relying on undocumented params or all rows including legacy power data should note the new filter and stricter strictV2 vs UI "certified".

Overview
Documents the public measured-power contract and adds optional row filtering on GET /api/v1/benchmarks.

OpenAPI and the /api reference now type all 15 POWER_METRIC_KEYS on BenchmarkRow.metrics, reserve optional power_invalid_reasons and power_audit, document previously implicit view/sequence params, and add a bilingual Measured power schema note. Constants introduce POWER_METRIC_KEYS as the single source for that surface (still folded into METRIC_KEYS without changing membership).

The route accepts powerValid (1, 0, any, strictV2; default/absent = any) and applies a post-cache filterByPowerValidity on non-calculator responses. Unknown values return 400; non-any values cannot combine with view=calculator (calculator rows drop power_valid). The stable API ledger and route digest are updated; Cypress and unit tests cover filters, docs parity, and backward-compatible behavior.

Reviewed by Cursor Bugbot for commit 07b9d25. Bugbot is set up for automated code reviews on this repo. Configure here.

…量:导出 POWER_METRIC_KEYS 功率契约指标键列表

Derives the full documented power surface (discriminators +
MEASURED_POWER_METRIC_KEY_LIST) as one exported constant and spreads it
into METRIC_KEYS, keeping set membership provably unchanged. The public
API documentation layer types BenchmarkRow.metrics from this export.
…enchmarks | API:为 /api/v1/benchmarks 新增 powerValid 实测功率有效性筛选参数

Pure post-cache row filter (cache keys unchanged): 1 keeps validated
rows, 0 keeps explicitly invalidated rows, any is the default identity
(legacy rows included), and strictV2 additionally requires
power_metric_schema_version == 2 (whole-deployment energy semantics).
Named strictV2 rather than certified to avoid colliding with the UI
tier's product meaning. Unknown values 400; the combination with
view=calculator 400s because the calculator cache stores rows already
trimmed past power_valid. Route digest bumped in the review ledger.
… | 文档:为 /api/v1/benchmarks 文档化实测功率契约

Types every POWER_METRIC_KEYS entry on the BenchmarkRow metrics schema
(additionalProperties still admits non-power keys), reserves the
forthcoming power_invalid_reasons / power_audit row fields from the
cross-plan producer contract, documents the previously-undocumented
view/sequence params plus the new powerValid param, adds a bilingual
measured-power schema note (tri-state power_valid, schema-version-2
whole-deployment energy, workers[] relationship, strictV2-vs-UI
divergence), and mirrors the parameter list in the stable contract
ledger. Cypress asserts the note and param render on /api and /zh/api.
…00 | 文档:说明 powerValid=any 可与 view=calculator 组合的例外

Review finding: the route only rejects non-any powerValid with
view=calculator, but the published copy claimed the combination is
always invalid. Align the operation description, the powerValid param
description, and the 400 response description (EN + ZH) with actual
behavior.
@edwingao28
edwingao28 requested a review from adibarra as a code owner August 28, 2026 02:55
@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
inferencemax-app Ready Ready Preview Aug 31, 2026 8:58pm

Request Review

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 2m 46s —— View job


Review complete — 🟢 LGTM

  • Read the diff against feat/etl-invalid-power-scrub
  • Review benchmark-power-validity.ts filter logic
  • Review route.ts integration + 400 handling
  • Review api-documentation.ts + constants changes
  • Check test coverage
  • Runtime verification on the dev server
  • Chinese copy review (review-zh-copy)

No blocking or warning issues found. This is a clean, backward-compatible, well-tested change.

What I verified at runtime (dev server, real read-only DB)

Check Result
powerValid=garbage 400 {"error":"Unknown powerValid filter"}
powerValid=1&view=calculator 400 {"error":"powerValid cannot be combined with view=calculator"}
powerValid=any&view=calculator 200 (no-op) ✅
absent vs powerValid=any byte-identical response ✅ (backward compat)
powerValid=1/0/strictV2 on current data 0 rows — every live row has power_valid: null (contract is forthcoming), and legacy rows correctly match only any
openapi.json powerValid enum ["1","0","any","strictV2"], metrics.power_valid typed, power_invalid_reasons reserved ✅

The unknown-value 400 short-circuits before the DB query, and the calculator-combination rejection is correctly ordered after it (an unknown value beats the combination error). Filter is a pure post-cache pass with no cache-key changes, so no blob-cache fragmentation.

Correctness notes

  • filterByPowerValidity matches the documented tri-state contract: === 1 / === 0 numeric checks align with the power_valid: numeric 1/0 producer contract, and strictV2 correctly requires power_metric_schema_version === 2. Rows without metrics or without a verdict fall through to any-only, as documented.
  • POWER_METRIC_KEYS refactor is a pure extraction — spread back into METRIC_KEYS, so set membership is provably unchanged and test-asserted (metric-keys.test.ts).
  • metricMapSchema is still used elsewhere (line 392), so the switch to benchmarkMetricsSchema on BenchmarkRow.metrics leaves no dead code.

Test coverage

Strong. New colocated benchmark-power-validity.test.ts (parser + filter, incl. certified non-aliasing and case-sensitivity), api-documentation.power.test.ts, 8 new route cases (filters, 400s, any≡absent compat, calculator no-op, agentic-metadata composition), and metric-keys.test.ts assertions pinning the 15-key contract.

Chinese copy

All six changed zh passages reviewed against their English sources — accurate and natural for the API-reference surface. No fidelity or naturalness findings; no maintainer escalation needed.

Note

Stacked PR — as the description states, this must land after #937 (feat/etl-invalid-power-scrub) and be retargeted to master. The dedicated tests-*/lint workflows did not appear in gh pr checks at review time (only Vercel + Claude + Bugbot); worth confirming they run green before merge, though the author reports all suites passing locally.

中文:清理内部规划标签和冗余叙述,并以当前契约语义明确功率审计字段说明。
中文:合并更新后的功耗剥离基础分支并保留公开 API 契约
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 2m 10s —— View job


Re-review — 🟢 LGTM

  • Read new commits since last LGTM review
  • Review filter logic + route integration
  • Review docs/constants/ledger changes
  • Check CI status
  • Runtime verification of powerValid filter + docs
  • Post review

What changed since the last review (f8fb515e07b9d25d): only the base-branch merge "chore: merge updated power scrub base", which brings comment simplifications in benchmark-mapper.ts / metric-keys.ts and removes some now-redundant test comments. Those files belong to the stacked base #937 and don't appear in this PR's own diff (origin/feat/etl-invalid-power-scrub...HEAD). No functional change to this PR since the previous approval. The powerValid filter, docs contract, and constants extraction are unchanged and remain correct.

Runtime verification (dev server, real read-only DB)

Check Result
powerValid=garbage 400 {"error":"Unknown powerValid filter"}
view=calculator&powerValid=1 400 {"error":"powerValid cannot be combined with view=calculator"}
view=calculator&powerValid=any 200 (no-op) ✅
absent vs powerValid=any byte-identical response ✅
openapi.json powerValid param enum + full description present ✅
/api + /zh/api "Measured power" / "实测功率" note both render ✅

The unknown-value 400 short-circuits before the calculator-combination 400 (verified), and the filter is a pure post-cache pass with no cache-key changes.

Notes

No blocking or warning issues found.
· feat/api-power-contract

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.

1 participant