refactor(ingestion): remove the superseded gold metric views - #2332
Merged
Conversation
The three data-quality tests are superseded by the observation-table checks, which cover the same measures at error severity instead of warn: git/collab/task nonnegative, plus the structural focus-hours bound in class_focus_metrics. Their gold source declared only those three views, so it goes with them. The seed's SYSTEM REFRESH existed to surface seeded rows in views that the unified path no longer reads; task_issue_state and task_status_spans are rebuilt by the gold run. refresh-task-views.sh had no automated caller, and fix-seed-aggregation.sql targeted a table since renamed. Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Metrics are served from the unified observation and evidence tables, so none of these 51 views has a reader: no gold model selects from one, the runtime only accepts relations ending in _metric_observations or _metric_evidence, and the query catalog that named them is write-only. The migration runner keeps no ledger and replays every file on each invocation, so deleting the files that create the views is the removal itself — a drop migration would only add another permanent replay. Remaining migrations cover identity DDL, class contract heals and the contract-version stamp; none references the insight database. Existing clusters keep the relations until dropped out of band; the applier never drops. Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
The snapshot is applied before migrations with CREATE OR REPLACE VIEW, so it would recreate every dropped view on each deploy until refreshed. staging.sql collapses to its CREATE DATABASE because dump-ddl derives it from the staging references in insight.sql, and the only one came from email_daily. Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
📝 WalkthroughWalkthroughThe change removes legacy ChangesLegacy Gold analytics removal
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
approved these changes
Aug 7, 2026
aleksdotbar
enabled auto-merge
August 7, 2026 10:07
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tools/seed/insight_seed/silver.py`:
- Around line 18-22: Remove the numbered apply-ch-migrations.sh workflow step
and its phase-ordering note from the module docstring in insight_seed; keep the
module header focused on the module’s purpose. Preserve the execution and
ordering contract in apply_ch_migrations()’s function docstring or a dedicated
runbook.
🪄 Autofix
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 Plus
Run ID: 4714ce25-a8b8-42d8-828b-8daf600c6532
📒 Files selected for processing (41)
src/backend/services/analytics/fix-seed-aggregation.sqlsrc/ingestion/dbt/tests/gold/assert_collab_messaging_bounds.sqlsrc/ingestion/dbt/tests/gold/assert_ic_chart_loc_nonneg.sqlsrc/ingestion/dbt/tests/gold/assert_ic_kpis_bounds.sqlsrc/ingestion/scripts/connectors-ddl/insight.sqlsrc/ingestion/scripts/connectors-ddl/staging.sqlsrc/ingestion/scripts/migrations/20260422000000_gold-views.sqlsrc/ingestion/scripts/migrations/20260422100000_ic-kpis-honest-nulls.sqlsrc/ingestion/scripts/migrations/20260422150000_team-member-honest-nulls.sqlsrc/ingestion/scripts/migrations/20260423120000_bullet-views-honest-nulls.sqlsrc/ingestion/scripts/migrations/20260427120000_views-from-silver.sqlsrc/ingestion/scripts/migrations/20260427180000_ai-bullet-rows-tool-filter.sqlsrc/ingestion/scripts/migrations/20260428000000_collab-metrics-update.sqlsrc/ingestion/scripts/migrations/20260429000000_task-delivery-silver-rewrite.sqlsrc/ingestion/scripts/migrations/20260430000000_git-bullet-expand.sqlsrc/ingestion/scripts/migrations/20260512000000_crm-gold-views.sqlsrc/ingestion/scripts/migrations/20260515000000_task-delivery-bullet-rewrite.sqlsrc/ingestion/scripts/migrations/20260518000000_collab-bullet-rewrite.sqlsrc/ingestion/scripts/migrations/20260519000000_ai-bullet-rewrite.sqlsrc/ingestion/scripts/migrations/20260520000000_code-quality-bullet-rewrite.sqlsrc/ingestion/scripts/migrations/20260521000000_git-bullet-rewrite.sqlsrc/ingestion/scripts/migrations/20260527000000_metrics-gold-views.sqlsrc/ingestion/scripts/migrations/20260601000000_ai-claude-team-metrics.sqlsrc/ingestion/scripts/migrations/20260604000001_drop-peer-cohort-stats.sqlsrc/ingestion/scripts/migrations/20260604000002_drop-ic-kpi-peer-median.sqlsrc/ingestion/scripts/migrations/20260605000001_ic-kpis-git-fanout-fix.sqlsrc/ingestion/scripts/migrations/20260609000000_ai-chatgpt-team-gold.sqlsrc/ingestion/scripts/migrations/20260610000000_ai-person-period-rollup-fix.sqlsrc/ingestion/scripts/migrations/20260611000000_support-bullet-rows.sqlsrc/ingestion/scripts/migrations/20260618000000_ai-claude-team-overage-gold.sqlsrc/ingestion/scripts/migrations/20260620000000_wiki-bullet-rows.sqlsrc/ingestion/scripts/migrations/20260623000000_ai_personal_gold_views.sqlsrc/ingestion/scripts/migrations/20260624000000_ic-chart-loc-git-breakdown.sqlsrc/ingestion/scripts/migrations/20260702000000_collab-person-counter-daily.sqlsrc/ingestion/scripts/migrations/20260708000000_task-delivery-status-category.sqlsrc/ingestion/scripts/post-deploy/refresh-task-views.shsrc/ingestion/silver/_shared/gold_sources.ymlsrc/ingestion/tools/seed/insight_seed/generators/hr.pysrc/ingestion/tools/seed/insight_seed/generators/people.pysrc/ingestion/tools/seed/insight_seed/generators/task.pysrc/ingestion/tools/seed/insight_seed/silver.py
💤 Files with no reviewable changes (37)
- src/ingestion/silver/_shared/gold_sources.yml
- src/ingestion/scripts/migrations/20260604000001_drop-peer-cohort-stats.sql
- src/ingestion/dbt/tests/gold/assert_ic_kpis_bounds.sql
- src/ingestion/dbt/tests/gold/assert_collab_messaging_bounds.sql
- src/ingestion/scripts/migrations/20260605000001_ic-kpis-git-fanout-fix.sql
- src/ingestion/scripts/migrations/20260422150000_team-member-honest-nulls.sql
- src/ingestion/scripts/migrations/20260624000000_ic-chart-loc-git-breakdown.sql
- src/ingestion/scripts/migrations/20260604000002_drop-ic-kpi-peer-median.sql
- src/ingestion/dbt/tests/gold/assert_ic_chart_loc_nonneg.sql
- src/ingestion/scripts/migrations/20260527000000_metrics-gold-views.sql
- src/ingestion/scripts/migrations/20260610000000_ai-person-period-rollup-fix.sql
- src/ingestion/scripts/migrations/20260618000000_ai-claude-team-overage-gold.sql
- src/ingestion/scripts/migrations/20260427120000_views-from-silver.sql
- src/ingestion/scripts/migrations/20260519000000_ai-bullet-rewrite.sql
- src/ingestion/scripts/connectors-ddl/staging.sql
- src/ingestion/scripts/migrations/20260428000000_collab-metrics-update.sql
- src/ingestion/scripts/migrations/20260620000000_wiki-bullet-rows.sql
- src/ingestion/scripts/migrations/20260423120000_bullet-views-honest-nulls.sql
- src/ingestion/scripts/migrations/20260515000000_task-delivery-bullet-rewrite.sql
- src/ingestion/scripts/migrations/20260609000000_ai-chatgpt-team-gold.sql
- src/ingestion/scripts/migrations/20260518000000_collab-bullet-rewrite.sql
- src/ingestion/scripts/migrations/20260623000000_ai_personal_gold_views.sql
- src/ingestion/scripts/migrations/20260702000000_collab-person-counter-daily.sql
- src/ingestion/scripts/migrations/20260611000000_support-bullet-rows.sql
- src/backend/services/analytics/fix-seed-aggregation.sql
- src/ingestion/scripts/migrations/20260422000000_gold-views.sql
- src/ingestion/scripts/migrations/20260429000000_task-delivery-silver-rewrite.sql
- src/ingestion/scripts/migrations/20260422100000_ic-kpis-honest-nulls.sql
- src/ingestion/scripts/migrations/20260520000000_code-quality-bullet-rewrite.sql
- src/ingestion/scripts/post-deploy/refresh-task-views.sh
- src/ingestion/scripts/migrations/20260430000000_git-bullet-expand.sql
- src/ingestion/scripts/migrations/20260521000000_git-bullet-rewrite.sql
- src/ingestion/scripts/migrations/20260601000000_ai-claude-team-metrics.sql
- src/ingestion/scripts/migrations/20260512000000_crm-gold-views.sql
- src/ingestion/scripts/migrations/20260427180000_ai-bullet-rows-tool-filter.sql
- src/ingestion/scripts/migrations/20260708000000_task-delivery-status-category.sql
- src/ingestion/scripts/connectors-ddl/insight.sql
Comment on lines
+18
to
+22
| 3. `apply-ch-migrations.sh` — applies migrations/*.sql (identity DDL, class | ||
| contract heals, the contract-version stamp), the staging label repair, | ||
| and `dbt run --select tag:gold` to build the dbt-owned gold models. Run | ||
| AFTER seeding so the materialized gold models are built over real seeded | ||
| silver instead of empty placeholders. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the workflow step from the module docstring.
This numbered apply-ch-migrations.sh step repeats the execution documented by apply_ch_migrations() and adds an AFTER seeding phase note to the module header. Keep the module docstring focused on module purpose. Retain the operational contract in the function docstring or a dedicated runbook.
As per coding guidelines, “Do not add module docstring headers that restate code, issue numbers, or phase/scope notes.”
🤖 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 `@src/ingestion/tools/seed/insight_seed/silver.py` around lines 18 - 22, Remove
the numbered apply-ch-migrations.sh workflow step and its phase-ordering note
from the module docstring in insight_seed; keep the module header focused on the
module’s purpose. Preserve the execution and ordering contract in
apply_ch_migrations()’s function docstring or a dedicated runbook.
Source: Coding guidelines
Gregory91G
added a commit
that referenced
this pull request
Aug 12, 2026
Three items in these documents pointed at files that no longer exist, and one argument rested on a surface that no longer does. `#2443` took `docs/components/connectors/ai/README.md` with the spec tree, and the parent metrics design with it, so the two late-phase items naming them are gone rather than repointed — neither describes work anyone can now do. `#2332` deleted the migrations that build the 51 legacy gold views, on the reasoning that none has a reader. One of them served `cc_overage` from `max(0, used − limit)`, which is the disagreement 18 was about. The section now says what is true: the surface is no longer created, the catalog entries naming it are inert, and the disagreement is latent — it returns only if something starts reading those relations again. The same removal took the migration the seat-dating convention was inherited from, so the decomposition states the convention itself instead of citing a deleted file. Signed-off-by: Gregory Gogin <grigoriy.gogin@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.
51
insight.*gold views left over from the pre-unified metric path. Nothing reads them.Dead how
523898d4;execute_metric_querygone.*_metric_observations/*_metric_evidence.Change
focus_time_pctbound is structural (dev_time_h = GREATEST(0, working_hours - meetings)).SYSTEM REFRESH(fed views nothing reads;task_issue_state/task_status_spansare rebuilt by the gold run),refresh-task-views.sh(no automated caller),fix-seed-aggregation.sql(targets a renamed table).CREATE OR REPLACE VIEW, so without this every deploy recreates them.Verified
dbt parseclean ·ruff check/formatclean · bootstrap 231 models PASS 0 ERROR · field parity 0 failures · snapshot diff pure deletion, 2 files, 0 additions · 16 gold tables intact.Not run: e2e metrics suite and
dbt build --selector data_quality(need a live stand).Follow-ups
PRD.md:182MUST keeps legacy gold ininsight, §4.5 sequences relocation-then-drop. Needs amending + contract version bump.metrics/metric_catalogtables and their seed migrations still there.🤖 Generated with Claude Code
Summary by CodeRabbit