test(e2e): metric-coverage gate + full bullet-stat assertions (p25/p75) with enforcement - #1511
Conversation
|
Warning Review limit reached
Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 64ecc5d5399242df511bd56fe012e031883c0591 and bd3d802. 📒 Files selected for processing (53)
📝 WalkthroughWalkthroughAdds a metric coverage gate and catalog snapshot artifact flow, tightens expectation checks so all returned stat fields must be asserted, and updates shared templates plus AI, collaboration, task-delivery, and wiki E2E fixtures to department-of-5 scenarios with empty-window cases. ChangesMetric Coverage Gate and E2E Fixture Refresh
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
2c4cc44 to
d4d4689
Compare
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
18b4223 to
e7af7e5
Compare
89f17a7 to
de6b0cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ingestion/tests/e2e/lib/expect_engine.py`:
- Around line 50-52: The stat detection in _stat_refs_in_cel is too broad
because it matches any whole-word stat token, even inside strings or on other
objects. Update _stat_refs_in_cel to detect only actual it.<stat> field access
in the CEL expression, and keep the result flowing into the no-unasserted-stat
check in the code that uses it. Use the existing _STAT_FIELDS and the caller
around the ledger update to ensure only real reads of it.value, it.median,
it.p25, etc. are counted as covered.
In `@src/ingestion/tests/e2e/lib/metric_coverage.py`:
- Around line 106-129: The skip hygiene logic only tracks explicit SKIP_LIST
entries, so SKIP_TABLES entries can become stale or fully redundant without
being flagged. Update the skip-gate flow around resolve_skips so table-based
skips are also included in the inputs checked by stale_skips and
redundant_skips, and make sure the baseline comparison treats SKIP_TABLES-backed
keys as first-class skip entries. Use the existing resolve_skips helper and the
stale_skips/redundant_skips call sites to locate the change and keep the
duplicate/doubly-specified validation intact.
- Around line 174-180: The asserted_keys_from_tests() scan is only looking at
top-level files with glob("*.test.yaml"), so nested metric fixtures under
subdirectories are missed. Update the file discovery in metric_coverage.py to
recurse through metrics_dir (and keep the existing parsing in
asserted_keys_from_tests()) so it matches the metrics/**/*.test.yaml layout and
collects metric_key values from all nested fixtures.
In `@src/ingestion/tests/e2e/metrics/collab_files_engaged.test.yaml`:
- Around line 30-57: The current m365_files_engaged fixture only validates
OneDrive aggregation, so it can miss regressions where SharePoint rows are
dropped or not summed with OneDrive. Update the collab_files_engaged test to add
a SharePoint-backed case or assertion alongside the existing
bronze_m365.onedrive_activity coverage, using the m365_files_engaged metric_key
and a query that verifies additive behavior across both products. Make sure the
new expectation is anchored in the existing test cases structure so it exercises
the same metric path with SharePoint data included.
In `@src/ingestion/tests/e2e/metrics/collab_files_shared_external.test.yaml`:
- Around line 29-56: The current `m365_files_shared_external` fixture only
exercises the OneDrive path, so it no longer covers the intended cross-product
aggregation. Add at least one SharePoint ingestion row and a corresponding
expectation in `collab_files_shared_external.test.yaml` using the same
`m365_files_shared_external` metric query, so `bronze_m365.sharepoint_activity`
is validated alongside `bronze_m365.onedrive_activity` and any additivity or
SharePoint regression is caught.
In `@src/ingestion/tests/e2e/metrics/collab_files_shared_internal.test.yaml`:
- Around line 29-56: The e2e case for m365_files_shared_internal only seeds and
checks bronze_m365.onedrive_activity, so it does not verify the SharePoint side
of the combined metric. Update this test to also seed SharePoint activity rows
alongside the existing OneDrive fixtures, and extend the expectation so
m365_files_shared_internal proves both sources are included and summed together
for the selected person. Use the existing collaboration query case and
metric_key assertion to keep the check in the same test while covering both
products.
In `@src/ingestion/tests/e2e/metrics/collab_meeting_hours.test.yaml`:
- Around line 28-53: The collab_meeting_hours test currently covers only the
Teams-backed `meeting_hours` path, so a Zoom or cross-source aggregation
regression could slip through. Update
`src/ingestion/tests/e2e/metrics/collab_meeting_hours.test.yaml` to keep at
least one `meeting_hours` assertion that exercises Zoom-backed or Teams+Zoom
additive behavior alongside the existing `collaboration` case, using the same
metric query/expect structure so the `meeting_hours` calculation is validated
across sources.
In `@src/ingestion/tests/e2e/metrics/collab_meetings_count.test.yaml`:
- Around line 18-24: The coverage note in this test is now stale because the
referenced additive proof no longer exists in collab_meeting_hours either.
Update the comment in collab_meetings_count.test.yaml to reflect the current
Teams-only fixture behavior, and add or point to a different test fixture that
actually exercises both Zoom and Teams together if cross-source additivity still
needs coverage. Use the existing metric test names and fixture setup in
collab_meetings_count and collab_meeting_hours to locate the affected assertions
and comments.
In `@src/ingestion/tests/e2e/metrics/wiki_outline_edits.test.yaml`:
- Around line 46-48: The wiki_edits expectation in this test fixture only checks
value, median, and range, so add explicit quartile assertions for the same
5-item distribution used elsewhere. Update the wiki metric block in
wiki_outline_edits.test.yaml to include p25 and p75 alongside the existing equal
checks, using the same quartile values that correspond to the {1,2,3,4,5}
dataset so the no-unasserted-stat gate is satisfied.
In `@src/ingestion/tests/e2e/metrics/wiki_outline_pages_created.test.yaml`:
- Around line 56-58: The wiki_pages_created metrics fixture is missing quartile
assertions for the selected wiki row, so update the expectation in the wiki
outline pages created test to include p25 and p75 alongside value, median,
range_min, and range_max. Use the existing metric_key match for
wiki_pages_created and align the expected quartiles with the underlying
{1,2,3,4,5} distribution so the new expect-engine checks pass.
In `@src/ingestion/tests/e2e/README.md`:
- Around line 95-99: Update the command examples in the README snippet to match
the current e2e workflow: the `./e2e.sh gates` example should describe the
metric-coverage gate only, and the `metric_coverage.py` example should use its
supported arguments instead of `--md`. Adjust the surrounding usage note so it
no longer implies all three gates run or that the ad hoc command will work with
the removed flag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7039e68d-dbbb-4918-b62d-b15e67bcfac3
📥 Commits
Reviewing files that changed from the base of the PR and between 1cb8c6a and de6b0cf0b734e831122f271e7c74baddf38aff58.
📒 Files selected for processing (53)
.github/workflows/e2e-bronze-to-api.ymlsrc/ingestion/tests/e2e/.gitignoresrc/ingestion/tests/e2e/README.mdsrc/ingestion/tests/e2e/conftest.pysrc/ingestion/tests/e2e/e2e.shsrc/ingestion/tests/e2e/lib/collect_metrics.pysrc/ingestion/tests/e2e/lib/expect_engine.pysrc/ingestion/tests/e2e/lib/metric_coverage.pysrc/ingestion/tests/e2e/meta/test_expect_engine.pysrc/ingestion/tests/e2e/metrics/ai_ai_loc_share2.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_active.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_cost.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_lines.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_overage.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_sessions.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_tool_accept.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_tool_acceptance.test.yamlsrc/ingestion/tests/e2e/metrics/ai_claude_web.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cursor_acceptance.test.yamlsrc/ingestion/tests/e2e/metrics/ai_prs_total.test.yamlsrc/ingestion/tests/e2e/metrics/ai_prs_with_cc.test.yamlsrc/ingestion/tests/e2e/metrics/collab_active_days.test.yamlsrc/ingestion/tests/e2e/metrics/collab_emails_read.test.yamlsrc/ingestion/tests/e2e/metrics/collab_emails_received.test.yamlsrc/ingestion/tests/e2e/metrics/collab_emails_sent.test.yamlsrc/ingestion/tests/e2e/metrics/collab_files_engaged.test.yamlsrc/ingestion/tests/e2e/metrics/collab_files_shared_external.test.yamlsrc/ingestion/tests/e2e/metrics/collab_files_shared_internal.test.yamlsrc/ingestion/tests/e2e/metrics/collab_meeting_free.test.yamlsrc/ingestion/tests/e2e/metrics/collab_meeting_hours.test.yamlsrc/ingestion/tests/e2e/metrics/collab_meetings_count.test.yamlsrc/ingestion/tests/e2e/metrics/collab_teams_chats.test.yamlsrc/ingestion/tests/e2e/metrics/collab_teams_meeting_hours.test.yamlsrc/ingestion/tests/e2e/metrics/collab_teams_meetings.test.yamlsrc/ingestion/tests/e2e/metrics/collab_zulip_chat.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_bugs_to_task_ratio_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_due_date_compliance_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_tasks_completed_jira.test.yamlsrc/ingestion/tests/e2e/metrics/team_bullet_collab_emails_sent.test.yamlsrc/ingestion/tests/e2e/metrics/team_bullet_task_delivery_tasks_completed.test.yamlsrc/ingestion/tests/e2e/metrics/templates/confluence_wiki_pages.yamlsrc/ingestion/tests/e2e/metrics/templates/jira_task.yamlsrc/ingestion/tests/e2e/metrics/templates/m365_email.yamlsrc/ingestion/tests/e2e/metrics/templates/outline_wiki_pages.yamlsrc/ingestion/tests/e2e/metrics/templates/people.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_active_authors.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_comments.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_edits.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_pages_created.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_active_authors.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_comments.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_edits.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_pages_created.test.yaml
|
@coderabbitai review The 5 actionable findings from the previous review are addressed in the latest commits:
The wiki-quartile comments were withdrawn (wiki returns no p25/p75). The SharePoint/Zoom cross-source comments are intentional per the single-scope simplification (see replies). |
|
✅ Action performedReview finished.
|
b145912 to
830a006
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/e2e-bronze-to-api.yml (1)
51-51: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable credential persistence on checkout.
Neither job pushes back to the repo; persisting the token in
.git/configfor the rest of the job is unnecessary exposure. Flagged by static analysis (artipacked).🔒 Proposed fix
- uses: actions/checkout@v4 + with: + persist-credentials: falseApply to both the
e2ejob (line 51) and themetric-coverage-gatejob (line 126).Also applies to: 126-126
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/e2e-bronze-to-api.yml at line 51, Disable credential persistence in the checkout steps used by the e2e-bronze-to-api workflow. Update both occurrences of actions/checkout@v4 in the e2e and metric-coverage-gate jobs to prevent the token from being stored in .git/config, since neither job needs to push changes back to the repository.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/e2e-bronze-to-api.yml:
- Line 51: Disable credential persistence in the checkout steps used by the
e2e-bronze-to-api workflow. Update both occurrences of actions/checkout@v4 in
the e2e and metric-coverage-gate jobs to prevent the token from being stored in
.git/config, since neither job needs to push changes back to the repository.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b412bb2c-b04a-4bd9-aee6-0eefa358d481
📥 Commits
Reviewing files that changed from the base of the PR and between b1459124aa9fdecd4022317841ec9a6d8980914e and 830a0061ea641ead74f711a398e785e3a4bafea7.
📒 Files selected for processing (53)
.github/workflows/e2e-bronze-to-api.ymlsrc/ingestion/tests/e2e/.gitignoresrc/ingestion/tests/e2e/README.mdsrc/ingestion/tests/e2e/conftest.pysrc/ingestion/tests/e2e/e2e.shsrc/ingestion/tests/e2e/lib/collect_metrics.pysrc/ingestion/tests/e2e/lib/expect_engine.pysrc/ingestion/tests/e2e/lib/metric_coverage.pysrc/ingestion/tests/e2e/meta/test_expect_engine.pysrc/ingestion/tests/e2e/metrics/ai_ai_loc_share2.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_active.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_cost.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_lines.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_overage.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_sessions.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_tool_accept.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cc_tool_acceptance.test.yamlsrc/ingestion/tests/e2e/metrics/ai_claude_web.test.yamlsrc/ingestion/tests/e2e/metrics/ai_cursor_acceptance.test.yamlsrc/ingestion/tests/e2e/metrics/ai_prs_total.test.yamlsrc/ingestion/tests/e2e/metrics/ai_prs_with_cc.test.yamlsrc/ingestion/tests/e2e/metrics/collab_active_days.test.yamlsrc/ingestion/tests/e2e/metrics/collab_emails_read.test.yamlsrc/ingestion/tests/e2e/metrics/collab_emails_received.test.yamlsrc/ingestion/tests/e2e/metrics/collab_emails_sent.test.yamlsrc/ingestion/tests/e2e/metrics/collab_files_engaged.test.yamlsrc/ingestion/tests/e2e/metrics/collab_files_shared_external.test.yamlsrc/ingestion/tests/e2e/metrics/collab_files_shared_internal.test.yamlsrc/ingestion/tests/e2e/metrics/collab_meeting_free.test.yamlsrc/ingestion/tests/e2e/metrics/collab_meeting_hours.test.yamlsrc/ingestion/tests/e2e/metrics/collab_meetings_count.test.yamlsrc/ingestion/tests/e2e/metrics/collab_teams_chats.test.yamlsrc/ingestion/tests/e2e/metrics/collab_teams_meeting_hours.test.yamlsrc/ingestion/tests/e2e/metrics/collab_teams_meetings.test.yamlsrc/ingestion/tests/e2e/metrics/collab_zulip_chat.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_bugs_to_task_ratio_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_due_date_compliance_jira.test.yamlsrc/ingestion/tests/e2e/metrics/task_delivery_tasks_completed_jira.test.yamlsrc/ingestion/tests/e2e/metrics/team_bullet_collab_emails_sent.test.yamlsrc/ingestion/tests/e2e/metrics/team_bullet_task_delivery_tasks_completed.test.yamlsrc/ingestion/tests/e2e/metrics/templates/confluence_wiki_pages.yamlsrc/ingestion/tests/e2e/metrics/templates/jira_task.yamlsrc/ingestion/tests/e2e/metrics/templates/m365_email.yamlsrc/ingestion/tests/e2e/metrics/templates/outline_wiki_pages.yamlsrc/ingestion/tests/e2e/metrics/templates/people.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_active_authors.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_comments.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_edits.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_pages_created.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_active_authors.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_comments.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_edits.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_pages_created.test.yaml
✅ Files skipped from review due to trivial changes (7)
- src/ingestion/tests/e2e/metrics/templates/outline_wiki_pages.yaml
- src/ingestion/tests/e2e/metrics/ai_prs_with_cc.test.yaml
- src/ingestion/tests/e2e/metrics/wiki_outline_active_authors.test.yaml
- src/ingestion/tests/e2e/metrics/ai_ai_loc_share2.test.yaml
- src/ingestion/tests/e2e/metrics/ai_claude_web.test.yaml
- src/ingestion/tests/e2e/metrics/templates/confluence_wiki_pages.yaml
- src/ingestion/tests/e2e/.gitignore
🚧 Files skipped from review as they are similar to previous changes (37)
- src/ingestion/tests/e2e/metrics/ai_cursor_acceptance.test.yaml
- src/ingestion/tests/e2e/metrics/ai_cc_cost.test.yaml
- src/ingestion/tests/e2e/metrics/ai_prs_total.test.yaml
- src/ingestion/tests/e2e/e2e.sh
- src/ingestion/tests/e2e/metrics/templates/m365_email.yaml
- src/ingestion/tests/e2e/metrics/collab_files_shared_internal.test.yaml
- src/ingestion/tests/e2e/metrics/ai_cc_overage.test.yaml
- src/ingestion/tests/e2e/meta/test_expect_engine.py
- src/ingestion/tests/e2e/metrics/collab_zulip_chat.test.yaml
- src/ingestion/tests/e2e/metrics/collab_active_days.test.yaml
- src/ingestion/tests/e2e/metrics/ai_cc_active.test.yaml
- src/ingestion/tests/e2e/metrics/templates/people.yaml
- src/ingestion/tests/e2e/metrics/ai_cc_tool_accept.test.yaml
- src/ingestion/tests/e2e/metrics/team_bullet_collab_emails_sent.test.yaml
- src/ingestion/tests/e2e/metrics/collab_emails_received.test.yaml
- src/ingestion/tests/e2e/metrics/collab_meeting_free.test.yaml
- src/ingestion/tests/e2e/metrics/ai_cc_sessions.test.yaml
- src/ingestion/tests/e2e/metrics/collab_meetings_count.test.yaml
- src/ingestion/tests/e2e/metrics/collab_emails_read.test.yaml
- src/ingestion/tests/e2e/metrics/collab_teams_chats.test.yaml
- src/ingestion/tests/e2e/metrics/templates/jira_task.yaml
- src/ingestion/tests/e2e/metrics/wiki_confluence_pages_created.test.yaml
- src/ingestion/tests/e2e/metrics/ai_cc_lines.test.yaml
- src/ingestion/tests/e2e/metrics/wiki_outline_edits.test.yaml
- src/ingestion/tests/e2e/metrics/ai_cc_tool_acceptance.test.yaml
- src/ingestion/tests/e2e/metrics/wiki_outline_pages_created.test.yaml
- src/ingestion/tests/e2e/metrics/collab_files_shared_external.test.yaml
- src/ingestion/tests/e2e/metrics/wiki_outline_comments.test.yaml
- src/ingestion/tests/e2e/metrics/collab_teams_meetings.test.yaml
- src/ingestion/tests/e2e/metrics/collab_files_engaged.test.yaml
- src/ingestion/tests/e2e/metrics/collab_teams_meeting_hours.test.yaml
- src/ingestion/tests/e2e/metrics/collab_meeting_hours.test.yaml
- src/ingestion/tests/e2e/metrics/wiki_confluence_edits.test.yaml
- src/ingestion/tests/e2e/metrics/collab_emails_sent.test.yaml
- src/ingestion/tests/e2e/metrics/wiki_confluence_comments.test.yaml
- src/ingestion/tests/e2e/lib/metric_coverage.py
- src/ingestion/tests/e2e/metrics/wiki_confluence_active_authors.test.yaml
Standalone CI gate (not the pytest suite) asserting every metric_id the analytics-api serves (GET /v1/metrics) is either exercised by a metrics/*.test.yaml or listed in an inline SKIP_LIST. Binary verdict: covered -> PASS, skip-listed -> PASS (baseline), neither -> FAIL; a stale/redundant skip also fails so the list cannot rot. - scripts/ci/metric_coverage.sh + compose.metric-coverage.yml: boot only MariaDB + analytics-api, read the universe over HTTP, run the diff. - .github/workflows/metric-coverage.yml: run on PRs (reuses build-images' analytics-api GHA layer cache for an incremental compile). - lib/metric_coverage.py: gate logic + inline SKIP_LIST (single SoT, no side-car file), URL-mode only. - Drain the reachable-todo backlog: add Team Bullet Collaboration (...0005) and Team Bullet Task Delivery (...0003) tests + bob/carol jira fixtures; remove both from SKIP_LIST. Both verified green through the e2e rig. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Fold the standalone metric-coverage.yml into e2e-bronze-to-api.yml: the coverage gate runs as a step after the suite, reusing the insight-e2e-analytics-api:local image `./e2e.sh build` already produces (no second analytics-api compile) and reading the universe from GET /v1/metrics. It runs even if the suite step failed, so both signals report. Delete the now-redundant standalone workflow. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
…number)
Switch the universe to POST /v1/catalog/get_metrics (96 product metric_keys,
each <table>.<column>) and count a key covered only when a test asserts its
VALUE (find:{metric_key} paired with equal/assert) — so every number on a
bullet is validated independently, not just "the query was called".
- universe_from_url: POST /v1/catalog/get_metrics, parse metrics[].metric_key.
- asserted_keys_from_tests: collect find.metric_key from value-checking rules.
- SKIP_LIST: now 78 metric_keys (was 34 metric_ids); bare<->dotted mapped by
the unique column suffix, with a collision guard.
- Report grouped by storage table. 18/96 value-tested today; the 15
"reachable: …" entries (13 jira + 2 zoom) are the backlog.
Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
… in report
render_markdown now leads with a coverage-by-vector summary table (friendly
vector names + %, sorted by coverage) and a "Reachable now" backlog list
(skips whose fixtures already exist), then collapses the full per-key detail.
Rows carry the catalog label ("Emails Sent") alongside the raw key. render_text
gains the per-vector + reachable-backlog summary too. No behavior change to the
verdict — display only.
Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
…" table
Per-key skipped rows now carry the real reason ("needs Bitbucket connector",
"reachable — Jira fixtures exist") as their status instead of a generic
"baseline", and the summary gains a "Skipped — by reason" breakdown (also in
the plain-text render). SKIP_LIST reasons rewritten to concise, status-ready
phrases. Display only — verdict logic unchanged.
Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
api/mod.rs: serve the in-process OpenAPI registry at GET /openapi.json
(public, merged after the tenant middleware like /health) — completes the
build_openapi follow-up TODO at mod.rs:81.
scripts/ci/openapi_spec.{py,sh}: a generator + drift-checker for the committed
docs/components/backend/analytics-api/openapi.json.
update -> rewrite the committed doc from the live route
check -> exit 2 + unified diff if it has drifted
URL mode needs only httpx; the .sh boots a throwaway MariaDB + analytics-api
(reusing scripts/ci/compose.metric-coverage.yml) or honors an existing
$ANALYTICS_API_URL. No silent defaults (code-conventions).
e2e-bronze-to-api.yml: add an 'OpenAPI spec drift gate' step (check) reusing
the built insight-e2e-analytics-api:local image — adding/changing a route
without regenerating the doc now blocks merge.
Regenerate openapi.json from the live router: 13 -> 21 operations. The
hand-maintained doc was stale — it was missing POST /v1/metrics/queries (the
endpoint the e2e suite hits most), POST /v1/catalog/get_metrics,
GET /v1/persons/{email}, and the 5 admin metric-thresholds routes.
Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Records every (method, path, status) the e2e suite exercises against
analytics-api and gates it against the committed OpenAPI spec — so a documented
route no test touches is surfaced and either gets a test or a SKIP_LIST entry
with a reason. Mirrors the metric-coverage gate's binary verdict + skip-list.
- lib/api_coverage.py: httpx response event-hook `record_response` (recording
half) + a pure-stdlib gate (comparison half, run as a plain file like
metric_coverage.py). Path-template matching for {id}/{email}/{table}; binary
verdict + SKIP_LIST(reason); reports which declared status codes each
exercised op actually validated.
- analytics_api.py: attach the hook in client() — the single chokepoint all
suite traffic flows through (metric tests via call_request, smoke direct).
- conftest.py: pytest_sessionfinish dumps the ledger to
.artifacts/observed_endpoints.json (host-visible via the /workspace mount).
- e2e-bronze-to-api.yml: 'API endpoint coverage gate' step — pure stdlib, reads
the ledger + committed spec, no Docker/network.
- .gitignore: ignore .artifacts/.
Suite exercises 3/21 ops today (GET /health, GET /v1/metrics,
POST /v1/metrics/queries); the other 18 are SKIP_LIST'd with reasons. Verified
offline (PASS/FAIL/template/hygiene paths) + live hook against a real
analytics-api (3/21 PASS).
Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
The workspace denies clippy::expect_used/unwrap_used and runs clippy with -D warnings, so the /openapi.json wiring failed 'Lint & Test': - doc_markdown: backtick `OpenAPI` in the openapi_info doc comment. - expect_used: router() now returns anyhow::Result<Router> and propagates the build_openapi error with ?; the sole caller (main.rs run_server) uses api::router(state)?. Fail-fast on a malformed spec instead of .expect(). Also refresh the stale 'not yet exposed over HTTP' registry comment. Compiles clean (cargo build); the e2e + arm64 builds already passed — this is the clippy-only path. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Minimise the Rust footprint of the /openapi.json route (issue constructorfabric#1505 item 1): revert router() to -> Router (and main.rs's api::router(state)) and handle build_openapi() inline with a match instead of threading anyhow::Result through the public signature + caller. On the (CI-caught, malformed-spec) error we tracing::error! and omit the route — no expect()/unwrap() (workspace denies both), no signature change. build_openapi can't be snapshotted at build time without constructing a full AppState (no test harness / sea-orm mock), so the runtime route stays the smallest change. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Stop booting a throwaway analytics-api for the metric + openapi gates. The e2e
suite already runs one — collect the gate inputs there and let the gates analyse
plain files as separate, lightweight, individually-requireable CI jobs.
Collection (during ./e2e.sh test, while analytics-api is up):
- api/test_coverage_artifacts.py snapshots GET /openapi.json -> openapi.live.json
and POST /v1/catalog/get_metrics -> catalog_metrics.json into .artifacts/, via a
RAW (un-hooked) httpx client so it does NOT pollute the endpoint ledger.
(observed_endpoints.json is still written by conftest.pytest_sessionfinish.)
Gate scripts gain file modes (no httpx, no Docker):
- metric_coverage.py: --universe-file <catalog_metrics.json>
- openapi_spec.py: check --live-file <openapi.live.json>
- api_coverage.py: already file-only
Workflow: 'e2e' builds + runs the suite + uploads .artifacts/ as coverage-inputs;
three gate jobs (needs: e2e) download it and analyse — metric-coverage-gate,
openapi-spec-drift-gate, api-endpoint-coverage-gate. CI now does ONE analytics-api
boot (the suite's), not three.
scripts/ci/{metric_coverage,openapi_spec}.sh keep their URL/boot modes for LOCAL
dev (e.g. openapi_spec.sh update to regenerate the committed doc).
Validated: collection emits valid artifacts; both file-mode gates pass against a
live analytics-api's snapshots (openapi drift OK, metric 18/96 PASS).
Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Two fixes after the artifact-gate run went red:
1. The 4-job run failed at the artifact handoff: upload-artifact@v4 defaults to
include-hidden-files:false and .artifacts/ is a dotdir, so it uploaded nothing
('No files were found') and all three gate jobs hit 'artifact not found'. Set
include-hidden-files:true (+ if-no-files-found:error so an empty collection
fails loudly at the source, not as three downstream errors). The suite,
collection, and ledger were all fine.
2. Collection is no longer a pytest test. Replace api/test_coverage_artifacts.py
with lib/collect_coverage_artifacts.py — a standalone script (--url/--out-dir/
--tenant) — run via subprocess from conftest's analytics_api fixture teardown,
while the API is still up (before proc.stop()), primary worker only,
best-effort. No forced analytics-api spawn for meta-only runs.
Validated locally: the script collects valid artifacts and both file-mode gates
pass against them (openapi drift OK, metric 18/96 PASS).
Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
The metric + openapi gates no longer boot a throwaway analytics-api — CI analyses the artifacts the e2e run already collects, so the separate stack is dead weight. - Delete scripts/ci/compose.metric-coverage.yml and scripts/ci/metric_coverage.sh (both only existed to boot that 2-service stack; CI uses --universe-file now). - e2e.sh: add './e2e.sh gates' — runs all three gates against .artifacts/ inside the runner image (--no-deps: no DB, no second compose). Local mirror of the CI gate jobs; run after './e2e.sh test'. - scripts/ci/openapi_spec.sh: boot-free — 'update'/'check' read the e2e-collected .artifacts/openapi.live.json via --live-file (pure stdlib, no Docker). For a running API, call openapi_spec.py --url directly. - Docs/messages: metric_coverage.py + openapi_spec.py docstrings, README gate section (now 'Coverage gates' — all three, artifact-based), and the workflow drift-gate hint point at the new flow. CI artifact mechanics unchanged (validated green on f92c2aea); this only removes the redundant local/CI compose boot. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
openapi_spec.sh was vestigial once the compose boot was gone — it only mapped update/check verbs, defaulted the .artifacts/ path, and cd'd to the repo root. Move all of that into openapi_spec.py: - modes are 'check' / 'update' (was the .sh's verbs; py's 'write' -> 'update'). - --file and --live-file default to repo-root-anchored paths (Path(__file__) .parents[2]), so it runs from any cwd; --live-file defaults to the e2e-collected .artifacts/openapi.live.json. - friendly 'run ./e2e.sh test first' error when the artifact is absent. Delete scripts/ci/openapi_spec.sh; repoint the workflow drift hint + README. CI's openapi gate job already calls the .py directly (unchanged). Validated: check/update, repo-root anchoring from another cwd, and the missing-file path. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
…structorfabric#5) The metric SKIP_LIST was 78 hand-maintained (key, reason) rows, ~59 of them mechanically 'this whole vector has no connector in the e2e rig'. Replace those with SKIP_TABLES: a {storage_table -> reason} map (7 rules) that auto-skips every universe key under a connector-less vector — so a new column there no longer fails the gate until someone hand-adds a row. SKIP_LIST keeps only the 19 per-key skips for MIXED tables (collab Slack/Zoom, task_delivery Jira backlog). resolve_skips(universe) merges both; redundant/stale hygiene now keys off the explicit SKIP_LIST entries only (a table-covered key that gets tested is just covered — its rule still covers the rest of the vector). Behavior-identical: validated against the live 96-key catalog → PASS 18/96, 78 baseline-skipped, 0 missing, same per-vector + by-reason counts. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
… code (constructorfabric#2, constructorfabric#4) constructorfabric#2 — the api-endpoint-coverage gate was 3/21 exercised with 18 PERMANENT skips (write/admin routes a read-only metric suite never hits) — a static allow-list, not drainable coverage. Remove the CI gate job. Keep the httpx recording hook + observed_endpoints.json (still collected + uploaded for observability); lib/ api_coverage.py still reports covered-vs-spec, and ./e2e.sh gates prints it non-blocking. CI is now 2 blocking gate jobs (metric + openapi-drift) + e2e. constructorfabric#4 — dead code: drop api_coverage.validated_count (unused, == len) and both gates' render_text (every caller used --md); markdown is now the only output, so drop the --md flag and its callers (workflow + e2e.sh gates). Validated: workflow parses (3 jobs), both gates render without --md (metric PASS 18/96, endpoint report PASS 3/21). Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
…eline (#1, constructorfabric#3) The drift check was a Rube Goldberg pipeline: serve /openapi.json -> boot the full stack -> collect openapi.live.json -> upload -> a separate CI job -> openapi_spec.py diffs. It's fundamentally one golden-file test, and the registry builds the doc with NO AppState (build_openapi runs before .with_state), so it needs no app boot. - api/mod.rs: split build_registry() out of router() (registry populated with no state); add #[cfg(test)] openapi_spec_matches_committed — canonical-sorts the built spec and diffs the committed docs/.../openapi.json. UPDATE_OPENAPI=1 cargo test … regenerates it. The runtime GET /openapi.json route stays (cheap, serves the contract per the gears comparison). Verified in rust:1.95: the canonical output is byte-identical to the committed doc, so the doc is unchanged. - Delete scripts/ci/openapi_spec.py; the drift gate now runs in backend-checks' cargo llvm-cov (which executes the suite). No artifact round-trip. - collect_coverage_artifacts.py: collect only catalog_metrics.json (no live spec). - e2e workflow: drop the openapi-spec-drift-gate job (constructorfabric#3 — now just e2e + metric-coverage-gate). e2e.sh gates + README repointed to the cargo test. Validated: cargo test (regenerate + verify) green; doc byte-identical; e2e workflow parses (2 jobs); collectors/conftest compile. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Drop the OpenAPI spec route/calc/gate and the endpoint-coverage gate added during review iterations — this PR is just the metric-coverage gate again. Removed: - analytics-api Rust /openapi.json route + build_registry split + the openapi_spec_matches_committed golden test (reverted api/mod.rs + main.rs to main); restored docs/.../openapi.json to main. - endpoint coverage: lib/api_coverage.py (deleted), the httpx recording hook in analytics_api.py (reverted), conftest.pytest_sessionfinish ledger dump. Kept (the metric gate): - lib/metric_coverage.py (SKIP_TABLES + SKIP_LIST, --universe-file). - lib/collect_coverage_artifacts.py — now snapshots ONLY catalog_metrics.json, invoked from conftest's analytics_api teardown (no second compose). - e2e workflow: e2e job (suite + upload the catalog) + metric-coverage-gate job. - ./e2e.sh gates runs the metric gate; the 2 Team-bullet metric tests + jira templates. Validated: workflow parses (e2e + metric-coverage-gate), scripts compile, no dangling refs to the removed features. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
Clearer name now that it snapshots only the metric catalog (the OpenAPI/endpoint collection was dropped). Updates the conftest helper (_collect_metrics) + its script path, the metric_coverage.py docstring, the workflow comments, and the README. No behaviour change. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
…omments Make the workflow comments self-contained: drop the 'same reasoning as <other-workflow>' cross-reference on the PR-only trigger and the speculative buildx-cache follow-up note. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
…serted-stat Redesign all 34 bullet metric fixtures to a 5-member department over a single custom window (+ an empty window), asserting every stat the served query returns: value/median/range_min/range_max/p25/p75 for collab and task_delivery, and the 4-stat subset for wiki and AI/CC (whose query_refs do not compute quartiles). Add a no-unasserted-stat gate to the expect engine: within a case, any find-matched row that carries a stat field must assert it (via equal or a CEL assert), so a fixture cannot silently skip a returned number. The gate is self-scoping — a stat absent from the response is never required. Make the equal check tolerant for fractional stats (math.isclose, abs_tol 1e-6) so a ratio/average is not rejected by binary float drift. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
The rebase pulled in upstream AI IC-bullet fixtures (ai_loc_share2, cursor_acceptance, claude_web, prs_total, prs_with_cc) that assert only value; the ...0013 bullet also returns median/range_min/range_max, so the no-unasserted-stat gate failed them. Assert the full 4-stat set: the department median/range for the two ratio bullets (verified against the served query), null for the three locked stubs. Skip-list collab_bullet_rows.slack_dm_ratio (needs Slack connector): the collab_zulip_chat single-metric redesign dropped its null assertion, leaving the key uncovered by the metric-coverage gate. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
… hygiene, recursive discovery - expect_engine: _stat_refs_in_cel counts only real it.<field> reads, so a stat name inside a string literal (e.g. it.metric_key == "p75") can no longer bypass the no-unasserted-stat gate (re.escape'd, hardcoded-safe pattern). - metric_coverage: flag a stale SKIP_TABLES vector (catalog serves no key under it); scan fixtures with rglob to match the rig's **/*.test.yaml discovery. - collab_meetings_count: correct the stale cross-source note (meeting_hours is Teams-only after the single-scope collapse). - README: fix stale gate command examples (single gate; drop removed --md flag) and the 18/96 -> 44/96 coverage figure. Self-scoping is unchanged: metrics whose response omits p25/p75 (wiki, AI) are still never required to assert them. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
… connector) Upstream's AI-personal-gold work (constructorfabric#1514) added the ai_person_counter_daily catalog vector — 10 per-person AI daily counters (accepted/removed lines, cost, active days, assistant messages/actions, chat/dev conversations, tool acceptance). Like ai_bullet_rows it needs a Cursor/Claude/ChatGPT connector to have data, so add it to SKIP_TABLES. Metric-coverage gate back to 0 missing (44/106). Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
64ecc5d to
bd3d802
Compare
What
Two complementary layers of e2e coverage for the analytics-api bullet metrics:
metric_keythe catalog exposes must have its value asserted by a test (or be explicitly skip-listed). A CI job, not a pytest test.value,median,range_min,range_max, and (where the served query computes them)p25/p75— enforced at runtime so a test can't silently skip a returned number.Breadth: the metric-coverage gate
Verdict is binary, per
metric_key(each number on a bullet is validated independently):metrics/*.test.yamlasserts its value (find: {metric_key: …}paired withequal/assert) → PASSSKIP_TABLES(a connector-less storage vector — e.g. allai_bullet_rows.*need a Cursor/Claude/ChatGPT connector) or an explicitSKIP_LISTentry → PASS (accepted baseline)A stale skip (key no longer in the catalog) or a redundant one (now value-tested) also fails, so the baseline can't rot.
How it runs (no second compose, no extra app boot): the universe is read from the live catalog (
POST /v1/catalog/get_metrics), not a fragile static parse of the seed migrations. Thee2esuite snapshots the catalog while analytics-api is already up (lib/collect_metrics.py, from theanalytics_apifixture teardown) into.artifacts/catalog_metrics.json, uploaded as thecoverage-inputsartifact. A separate pure-Pythonmetric-coverage-gatejob reads that file (--universe-file) + the test YAMLs and renders the verdict — no Docker, no analytics-api boot in the gate.Coverage today: 44/96
metric_keys value-tested · 52 baseline-skipped · 0 missing → PASS.Depth: full bullet-stat assertions (p25/p75) + runtime enforcement
All 34 bullet-metric fixtures assert the complete stat set the API returns:
p25/median/p75are interior quartiles (not collapsed to min/max), seeded over a single custom window + an empty window (proves the date bound excludes out-of-range rows).value/median/range_min/range_max/p25/p75for collab & task_delivery; the 4-stat subset for wiki & AI/CC, whose servedquery_refs don't compute quartiles.No-unasserted-stat enforcement (
lib/expect_engine.py): within a case, anyfind-matched row that carries a stat field MUST assert it (viaequalor a CELassert) — otherwise the rule fails. It is self-scoping: a stat absent from the response is never required, so the 4-stat vectors pass withoutp25/p75, and empty-window cases (no matched row) are exempt. Theequalcheck compares numbers with tolerance (math.isclose,abs_tol=1e-6) so a fractional stat (ratio / average) isn't rejected by binary float drift, while a real ≥1e-4 difference still fails.Notable findings (worked around in fixtures; no backend changes)
p25/p75) — the self-scoping gate handles this uniformly.bronze_zoom.participantsisReplacingMergeTree ORDER BY email, so a person collapses to one Zoom row regardless of distinct meetings →meetings_countis Teams-only (cross-source proof lives inmeeting_hours).silver.class_*is truncated only at session start, not per-test → a fixture's rows can leak into the next; wiki fixtures use disjoint per-fixture windows so any leaked foreign row falls outside the window (zero rig change).Contents
lib/metric_coverage.py— gate logic +SKIP_TABLES(7 vectors) /SKIP_LIST(20 per-key; single source of truth, no side-car).lib/collect_metrics.py— live-catalog snapshot script.lib/expect_engine.py— no-unasserted-stat enforcement + float-tolerantequal.metrics/*.test.yamlredesigned to the full-stat / single-custom-window shape, + 4 shared templates (5-member departments)..github/workflows/e2e-bronze-to-api.yml(thee2ejob uploads the catalog; themetric-coverage-gatejob analyses it),conftest.py,e2e.sh(./e2e.sh gates),README.md,.gitignore.Validation
Local:
./e2e.sh test(snapshots the catalog) →./e2e.sh gates.Summary by CodeRabbit