test(data): silver business-rule data tests — non-negative counts (#1321)

[SharedQA]

What

First custom singular data test for the silver layer (the feedback's section 4): assert_collab_document_counts_non_negative.sql — activity counts on class_collab_document_activity can never be < 0. dbt fails the build if any row violates it; honest NULLs (NULL < 0 → NULL) are not flagged.

Why

A negative count = a broken transform or bad source row, and it silently corrupts any metric that sums it. This is the verifiable, no-warehouse-needed slice of the silver-integrity work under EPIC #1321 / #1323.

Scope / next

• Now: the pattern, on the collab document model (columns confirmed).
• Rollout: same non-negative + date-sanity + accepted_values tests across the other class_* tables.
• Deferred (needs warehouse-gated CI): enforced contract: {enforced: true} with data_type — the union models' exact ClickHouse types can't be validated without a dbt build, so they land once data-checks.yml runs against a populated ClickHouse. Authoring them blind would risk a red build on a wrong type.

Draft until the dbt CI run validates it (dbt isn't runnable in this environment).

Summary by CodeRabbit

• Tests
  • Added a new data-quality validation for collaboration document activity metrics. The test monitors document viewing, editing, syncing, sharing, and page visit counts to ensure they remain non-negative. When potential data anomalies are detected, detailed failure information is automatically logged for investigation, enhancing overall data integrity and system reliability across collaboration features.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 06e33b3d-c446-42df-8011-c0a1da93bc93

📥 Commits

Reviewing files that changed from the base of the PR and between c32821d and 6b09a29.

📒 Files selected for processing (1)

• src/ingestion/dbt/tests/collaboration/assert_collab_document_counts_non_negative.sql

---

📝 Walkthrough

Walkthrough

A new dbt singular test SQL file assert_collab_document_counts_non_negative.sql is added under the collaboration tests directory. It is configured with a data_quality tag, severity='warn', and store_failures=true, and queries class_collab_document_activity to surface rows where any of five activity-count columns are negative (< 0), leaving NULL values unaffected.

Changes

Non-negative collab document counts data-quality test

Layer / File(s)	Summary
dbt test config and validation query src/ingestion/dbt/tests/collaboration/assert_collab_document_counts_non_negative.sql	Adds the full singular test: dbt config block sets data_quality tag, warn severity, and failure storage; the query selects unique_key, date, and all five count columns from class_collab_document_activity FINAL where any count is < 0.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 Hop hop, I check each count,
No negatives allowed to mount!
With FINAL clause and < 0 guard,
My data stays pure — not scarred.
NULL slips by, no fuss today,
Just negatives I keep at bay! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: adding a silver-layer data quality test for non-negative counts in the collaboration document activity table, directly addressing the changeset's content.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[SharedQA]

Two things changed in the last commit, and the reasoning behind each:

1. Wrapped the test in a config block so a failure is operationally useful, not just noise

• store_failures=true — when a negative count turns up, dbt writes the offending rows into an audit table instead of only logging an error. You can then SELECT those rows and trace each one back to the feeder mapping, rather than re-running to find them.
• severity='warn' — the check warns instead of hard-failing the build, so a single bad row doesn't halt the pipeline and starve downstream dashboards, while still surfacing as a finding for the team.
• meta (title / domain / category / tier / remediation) — so when it fires, the alert carries the fix instructions with it; whoever responds doesn't have to guess what the test meant or how to act on it.

This also matches the repo's documented data-quality convention (tests/README.md) rather than being a one-off blocking test.

2. Read the table FINAL (ClickHouse specific)

class_collab_document_activity is a ReplacingMergeTree. On update, ClickHouse keeps the old row and merges away the duplicate in the background later. Without FINAL, a plain read can briefly see both the old (broken) row and the new (fixed) row before that merge runs, which would report a phantom failure. FINAL resolves the duplicates at query time, so the test only ever evaluates the latest true state of each row. It's also the convention for reading RMT tables in this repo.

[SharedQA]

Review feedback — disposition

Thanks for the thorough review. Triaged the three points; one applied, two intentionally not:

1. Add date to the SELECT — ✅ applied (a803a97)
Included date in the stored-failure rows so on-call gets the activity day of the offending count directly in the audit schema, no second lookup. Kept as triage context only, not a filter.
Note: the cited performance rationale (partition pruning) doesn't apply to this table — class_collab_document_activity is ReplacingMergeTree(_version) ordered by unique_key with no partition_by, so a unique_key lookup already hits the sort key. The value here is human triage, not planner efficiency.

2. Window FINAL to the last 7 days — ⛔ intentionally not done
This is a hard physical-bound invariant (tier=error): a negative count is corruption wherever it lands, including older partitions rewritten by a late backfill or a historical transform bug. A date >= today() - 7 window would deliberately blind the check to exactly those cases. Completeness wins over speculative FINAL cost on a correctness invariant. If FINAL ever proves measurably expensive, the correct bound is arrival time (collected_at / _version), not activity date — bounding by activity date reintroduces the late-backfill blind spot (the same one addressed in the #1355 spike-check follow-up by switching to collected_at).

3. "Silent drop" upstream blindspot — ✅ acknowledged, out of scope for this file
Correct: a dbt test on the silver output can't catch an upstream GREATEST(0, …) clamp or a WHERE count >= 0 drop. That's covered by the broader strategy — pair this with a pytest e2e test using a known-bad bronze seed (part of #1321), not something fixable in this SQL.