feat(analytics-api): collab Messaging query + catalog seed + e2e (#1527 B+C) - #1580
Merged
Conversation
|
Warning Review limit reached
Next review available in: 14 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: 📒 Files selected for processing (6)
✨ 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 |
ktursunov
previously approved these changes
Jul 3, 2026
mozhaev-dev
force-pushed
the
feat/collab-person-counter-scaffold
branch
from
July 6, 2026 08:34
f6e5a11 to
f7d323a
Compare
mozhaev-dev
force-pushed
the
feat/collab-messaging-backend-e2e
branch
from
July 6, 2026 08:34
09c21e5 to
28cf87b
Compare
mozhaev-dev
force-pushed
the
feat/collab-person-counter-scaffold
branch
from
July 6, 2026 11:59
333d99f to
952394b
Compare
mozhaev-dev
force-pushed
the
feat/collab-messaging-backend-e2e
branch
2 times, most recently
from
July 6, 2026 12:42
5efa499 to
b5ef2c7
Compare
mozhaev-dev
changed the base branch from
feat/collab-person-counter-scaffold
to
main
July 6, 2026 12:42
…ed (#1527) PR B of the #1527 split — the backend layer over PR A's gold view. Clones the - Query_ref migration (m20260702_000001): `collab_messaging_counters_qr` reads insight.collab_person_counter_daily, re-aggregates per person with the honest-NULL `if(countIf(x IS NOT NULL) > 0, sumIf …, NULL)` wrapper, ARRAY JOIN unpivot (drops NULLs), and LEFT JOINs per-org_unit_id `quantileExact` bands. Stored on metrics id …0053 (next free after AI counters …0052). - Catalog seed (m20260702_000002): product-default metric_catalog rows for `messages_sent` + `channel_posts`, each with an initial product-default metric_threshold (catalog-probe requirement) and a metric_query_catalog junction link to the query. source_tags carry the vendors (m365/slack/zulip). Thresholds are initial defaults, NOT calibrated (per-org_unit calibration is a post-release follow-up). - Register both migrations in mod.rs. Tests: - Rust unit (feeds #1433): asserts the generated peer-counter SQL emits both keys with the honest-NULL wrapper + per-org_unit band join — no DB, runs in CI coverage. - Extend catalog live_tests (`product_default_wins_when_no_tenant_overlay`) to assert both Messaging keys resolve at product-default. Designed for extension: later modalities add keys to the ARRAY JOIN list + seed rows. Depends on the gold view from PR A (#1567) at deploy time. Honest-NULL rendering is #1517. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…1527) PR C of the #1527 split — end-to-end YAML specs over the gold view (PR A) and peer-counter query + catalog seed (PR B). One metric_key per file (current rig convention, à la collab_emails_{sent,received,read}); both query metric_id …0053. - collab_messaging_messages_sent.test.yaml: department of 5 with per-person totals {10,20,30,40,50} sourced across Teams + Zulip to prove the cross-vendor UNION — carol = Teams 15 + Zulip 15 = 30. Cases: cross-vendor sum · single- vendor still non-NULL (bob, Zulip-only) · empty window → NULL, not 0. - collab_messaging_channel_posts.test.yaml: department of 5 with per-person Teams post+reply counts summing to {2,4,6,8,10}, proving the posts+replies fold (carol = post 4 + reply 2 = 6). Cases: fold · empty window → NULL, not 0. Reuses existing people / m365_teams / zulip templates + schemas; both specs pass static validation (ref resolution + schema padding + additionalProperties:false). Scope notes: - Slack is a declared source but has no e2e fixtures yet (needs schemas/bronze_slack.users_details.yaml + a template from the real bronze). The multi-vendor honest-NULL contract is proven with Teams + Zulip; adding Slack is a follow-up. - Depends on PR A (gold view) + PR B (query_ref + seed) being present for the full ClickHouse run; not runnable green until those land. Honest-NULL "No data"/tooltip rendering is #1517. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
The single-vendor non-NULL case asserted only `value`; the row also carries the department bands, which are deterministic (same org_unit distribution as the carol case). Assert median/p25/p75/range too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
mozhaev-dev
force-pushed
the
feat/collab-messaging-backend-e2e
branch
from
July 6, 2026 12:45
b5ef2c7 to
c5f1549
Compare
ktursunov
approved these changes
Jul 6, 2026
ktursunov
pushed a commit
to ktursunov/insight
that referenced
this pull request
Jul 8, 2026
…ric gate The collab Messaging tests (constructorfabric#1527) assert the response metric_key the unified-metrics path returns — the full dotted catalog key (collab_person_counter_daily.messages_sent) — but the gate's resolver only understood bare column keys mapped by suffix, so both messaging keys were flagged MISSING and the metric-coverage gate has been red since constructorfabric#1580 merged. A dotted assertion now matches the catalog directly; bare keys keep the suffix mapping. Verified: 46/108 validated, 0 missing. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidated PR B+C of the #1527 split — merges the former #1568 (backend query_ref + catalog seed) and #1578 (e2e YAML) into one unit, stacked on #1567 (gold view + dbt).
Why combined
The new Metric coverage gate couples catalog-seed ↔ e2e-test: a seeded
metric_keywith no value-test fails ("missing"), and a test asserting an unseeded key fails ("not a catalog metric_key"). Split across two PRs, #1568 and #1578 could each never go green independently (mutual dependency). The E2E suite additionally needs the gold view from #1567. Landing seed + query + test together (on top of the view) makes both gates + E2E green on one branch.Contents
m20260702_000001_collab_messaging_queries—collab_messaging_counters_qr(honest-NULL wrapper + per-org_unit_idquantileExactbands),metrics.id …0053. Rust unit test on the generated SQL (feeds test(analytics-api): unit suites for infra/identity, admin_threshold, api + auth #1433).m20260702_000002_seed_collab_messaging_catalog— product-default rows formessages_sent+channel_posts+ thresholds +metric_query_cataloglinks;source_tagscarry the vendors.domain/catalog/live_tests.rsto assert both keys resolve at product-default.collab_messaging_messages_sent(cross-vendor SUM · single-vendor non-NULL · empty→NULL) andcollab_messaging_channel_posts(posts+replies fold · empty→NULL). Bob-case stat assertions completed per CodeRabbit on test(e2e): collab Messaging metrics — messages_sent, channel_posts (#1527) #1578.Review status carried over
metric_query_catalog(metrics_id, metric_catalog_id)UNIQUE (uq_metric_query_catalog_pair) confirmed →INSERT IGNOREseed is idempotent.Notes
slack_*as "needs Slack connector"); honest-NULL contract proven with Teams + Zulip.mainwhen feat(gold): collab_person_counter_daily scaffold + Messaging counters (#1527) #1567 merges.Supersedes #1568 and #1578. Refs #1527 · #1516
🤖 Generated with Claude Code