Skip to content

MET-WP4-WIDGET-01: public metricWidget read, off by default - #1011

Draft
ja573 wants to merge 10 commits into
developfrom
feature/metrics-widget/met-wp4-widget-01
Draft

ja573 wants to merge 10 commits into
developfrom
feature/metrics-widget/met-wp4-widget-01

Conversation

@ja573

@ja573 ja573 commented Nov 10, 2026 •

Copy link
Copy Markdown
Member

Implements MET-WP4-WIDGET-01 (#1007): a public, read-only metricWidget GraphQL query for the embeddable metrics widget. It ships inactive behind THOTH_METRIC_WIDGET (default OFF). Authority: ADR-0014 (#1008, merged as cab25eba). Spec: the #1007 body, R10 (APPROVED). Grant: #1007 comment 6777125645.

What it does

  • metricWidget(input: { doi, chapterDois, startDate, endDate }) reads one work and its child works, for publishers whose package grants METRICS_WIDGET. It serves only registry-public, enabled and additive measures, monthly, with up to 12 months per read and no credential.
  • The Metrics engine gains pair mode and an optional read deadline (metric_dashboard/mod.rs, Rust parameters only). The dashboard path, its SQL and its tests are unchanged.
  • Cost bounds: 2 concurrent widget reads per process, one per HTTP request, a 1 s checkout, a 5 s statement timeout, a 10 s deadline, 500 child works and 40 pairs per section. Nothing is truncated.
  • Refusals use one uniform METRIC_WIDGET_UNAVAILABLE. Widget errors use fixed widget-owned messages, never an engine message, SQL or package value.
  • Docs: contract register section 4.3, the METRICS_WIDGET description (ADR-0014 s4.2), and two clauses in docs/metrics/README.md and contracts.md (granted budget addition). One CHANGELOG line.

Base and commits

  • Base develop @ cab25eba. Head b2cb0730, ten commits (five implementation commits plus five review-fix commits), all authored and committed by Javier Arias.
  • 21 files, all on the spec's section 9 budget. metric_dashboard/tests.rs is unmodified. Three existing assertions change, as the spec names.

Review fixes (exact-head reviews at f58d0905, both CHANGES REQUIRED)

  • CI lint failed at f58d0905: the runner's newer stable Rust deprecates AtomicUsize::fetch_update, which local 1.97.0 does not flag. The slot limiter now uses a compare_exchange_weak loop.
  • A repeated child row is refused as INTERNAL_ERROR (impossible under work_relation_relator_related_uniq, now explicit).
  • New tests: an unserved cross pair beside served pairs equals single-pair dashboard reads; the deadline passing between engine statements is BUSY; the frontier check. One test comment, the contracts.md wrap and the implementation report corrected.

Local validation at b2cb0730 (implementation report in the PR)

  • cargo fmt --all -- --check passes. cargo clippy --all --all-targets --all-features -- -D warnings passes on rustc 1.97.0 and on 1.99.1.
  • cargo test --workspace passes against a local test database (thoth_api lib 2222 passed, 3 ignored; 53 model and 9 GraphQL widget tests).
  • Statement ceiling: 19 named statements (22 captured), pinned by a test.

Not in scope

  • Release, deploy, activation (CRITICAL unless the CTO decides otherwise, ADR-0014 s4.7), the WAF stage, MET-READ-CACHE-01, MET-WP4-WIDGET-ALLTIME, and the widget package change (WIDGET-02).

Gates

  • Delta review at the new head by Opus 5.5 at max effort and a fresh Fable, then a CTO MERGE AUTH and a merge-commit landing with the expected head.

Generated by Claude Code

ja573 commented Nov 10, 2026

Copy link
Copy Markdown
Member Author

CI lint fails at f58d0905: cargo clippy ... -D warnings on the runner's current stable Rust reports AtomicUsize::fetch_update as deprecated (renamed try_update) at thoth-api/src/model/metric_widget/mod.rs:833, in try_take_slot. The local toolchain (1.97) does not deprecate it, which is why the local clippy run passed. This is this PR's code.

Fix: replace fetch_update with an equivalent compare_exchange_weak loop. It behaves the same and compiles warning-free on both toolchains (try_update does not exist on older stable releases). It is not pushed yet because the two exact-head reviews are running against f58d0905. It will go in one push with any review findings, followed by a delta review at the new head.


Generated by Claude Code

@ja573 ja573 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

REVIEW MET-WP4-WIDGET-01 @ f58d090: CHANGES REQUIRED

Reviewer: fresh Claude Opus 5.5 (max), HIGH leg, independent of the Fable leg. Read-only.
Binding: head, tree 36d5193c, base cab25eba and spec R10 sha256 6fa77f37… match; 5 commits authored and committed by Javier Arias, no trailers; metric_dashboard/tests.rs blob unchanged.
Checks: budget, anonymous boundary and bypasses, cost bounds, dashboard unchanged, correctness against s3 and s4, tests and docs PASS; CI FAIL (lint).
Tests (head-built binaries, local database): metric_widget 58 passed; metric_dashboard 87 passed, 1 ignored; thoth_api lib 2218 passed, 3 ignored (3 Redis failures were a full-disk fault and passed on a fresh Redis); other crates pass. fmt passes. Local clippy not run; rustc 1.97.0 would not flag F1.
Findings: F1 (P1) CI lint fails on deprecated AtomicUsize::fetch_update (metric_widget/mod.rs:833); use a compare_exchange_weak loop, no allow. F2 (P2) the equivalence oracle never has an unserved cross pair; add a diagonal section. F3 (P3) the engine deadline is tested only at the first engine statement; add an ACCOUNTS_SQL case. F4 (P3) tests.rs:518 comment says "No database". F5 (P3) the report's clippy and CI wording. F6 (P3 note) the #1008 CHANGELOG wording, at release prep.
Implementer's four spec findings: agreed, none blocking.


Generated by Claude Code

@ja573 ja573 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

REVIEW MET-WP4-WIDGET-01 @ f58d090: CHANGES REQUIRED

Reviewer: fresh Claude Fable 5.2, HIGH leg, independent of the Opus leg. Read-only.
Binding: head, tree 36d5193c, base cab25eba, clean worktree, spec R10 sha256 6fa77f37… and grant 6777125645 match.
Checks by reading: budget, identity, anonymous boundary, cost bounds, dashboard unchanged, contract and docs in order. cargo fmt --all -- --check passes. No local clippy or tests ran (environment), so this leg has no test counts.
Findings: F-01 (P1) CI lint fails on deprecated AtomicUsize::fetch_update (metric_widget/mod.rs:833). F-02 (P2) the 500-children bound counts children after deduplication, so a duplicated has-child row could hide a truncated set. F-03 (P2) the equivalence oracle never tests a grid smaller than the cross product (tests.rs:2166-2283). F-04 (P3) no FrontierMismatch test. F-05 (P3) contracts.md:82 not re-wrapped. F-06 (P3) orchestrator-side report commands lack --workspace. F-07 (P3) in-repo report says "CI status: PENDING".
Implementer's four spec findings: accepted (the fourth as a note).


Generated by Claude Code

@ja573 ja573 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

REVIEW MET-WP4-WIDGET-01 @ b2cb073: APPROVED

Reviewer: fresh Claude Fable 5.2, HIGH leg, delta review of the fix pass from f58d0905, independent of the Opus leg. Read-only.
Binding: PR head, worktree HEAD, tree 6100aa08, base cab25eba and spec R10 sha256 6fa77f37… match; the delta touches 4 budget files; new commits authored and committed by Javier Arias, no trailers.
Local (rustc 1.97.0): cargo fmt --all -- --check and cargo clippy --all --all-targets --all-features -- -D warnings exit 0; cargo test --workspace exit 0 (thoth_api lib 2222 passed, 3 ignored); widget model 53, widget GraphQL 9, dashboard model 67 (1 ignored), dashboard GraphQL 20. CI at this head: all ten checks passed.
Prior findings: all resolved or correctly ruled. Fable F-02's premise was false (work_relation_relator_related_uniq); the explicit repeated-row refusal cannot hit valid data and adds no message, code or log field. The diagonal oracle, the deadline-between-statements test and the frontier test cannot pass for the wrong reason. Opus F6 is left for release preparation.
New (all P3, no change before merge): N-01 the deadline test has two timing windows, so a slow runner could fail falsely, never pass falsely (7 of 7 here). N-02 the frontier test copies the prelude and label binding from transact. N-03 PR body footer is not a commit trailer.
Scope note: this reviews the head on cab25eba. B2 has since landed (HOLD 6781693857); the refresh gets its own review.


Generated by Claude Code

@ja573 ja573 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

REVIEW MET-WP4-WIDGET-01 @ b2cb073: CHANGES REQUIRED

Reviewer: fresh Claude Opus 5.5 (max), HIGH leg, delta review of the fix pass from f58d0905, independent of the Fable leg. Read-only.
Fix pass: correct. All 13 prior findings are resolved, confirmed or deferred as ruled. The orchestrator's F-02 ruling holds (work_relation_relator_related_uniq is unique and not deferrable). The new tests cannot pass for the wrong reason. CI lint now passes on the runner that failed. fmt and clippy pass; widget model 53, widget GraphQL 9, dashboard model 67 (1 ignored), dashboard GraphQL 20, registry 12.
D2 (P1, the HOLD in 6781693857): B2 (#1010) merged into develop as af6719f6 during this review, so b2cb0730 no longer merges. There are 21 conflict blocks in metric_dashboard/mod.rs and 1 in the contract register, where both PRs add a section 4.3. B2 rebuilt the read path pair mode is built on. It added a 3-read process limit, a 20 s deadline and MetricReadError::Busy, and made private again engine constants the widget imports. Fix: refresh onto af6719f6 with the same authorization cab25eba had. Rebuild pair mode on the new read path. Settle and record the widget's relation to B2's limit, deadline and Busy against spec s4 and s10, or amend the spec. Renumber the register section, then run the full tests, CI and an exact-head review of the resolution.
D1 (P3): in the deadline test (metric_widget/tests.rs:1511-1523), move capturing_pool() above hold_lock, and optionally sleep 2 s instead of 1.5 s.


Generated by Claude Code

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