seed: build silver+gold via real deploy scripts; fix gold data conformance - #1758
Conversation
…mance Table/gold setup now uses the same mechanism as a real deployment instead of a Python reimplementation that skipped the dbt-owned gold layer. silver.py runs create-bronze-placeholders.sh, then seeds, then apply-ch-migrations.sh (migrations + `dbt run --select tag:gold`), then refreshes the task refreshable MVs. Seeding sits between the two scripts so the materialized git_metric_observations table is built over real rows. - Dockerfile: pin python:3.12-slim (matches toolbox), add bash; dbt-clickhouse added to requirements.txt. - docker-compose.yml: mount full ./src/ingestion at /ingestion, set CLICKHOUSE_DATABASE, redirect dbt target/logs to /tmp. - clickhouse-user-defaults.xml: enable allow_experimental_refreshable_materialized_view (the migrations SET it per-statement but run_ch is stateless; production CH allows it server-side). Generator conformance so the gold observation relations populate: - git.py: PR state MERGED/OPEN; add tenant_id + data_source; commit data_source. - ai.py: assistant surface chat; add conversation_count + tool/surface labels. - people.py: seed workspace_id (was collapsing metric_entity_cohorts). - task.py: seed class_task_statuses dimension + carry status_id on status events (fixes jira_closed_tasks = 0). - base.py: deterministic_int for Int64 id columns (pr_id). Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
📝 WalkthroughWalkthroughThe PR aligns local seed execution with ingestion deployment scripts, expands seeded silver-table data, adds task status dimensions, updates container and ClickHouse defaults, and introduces a ChangesSeed pipeline
Compose service URL reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SeedRunner
participant CreateBronzePlaceholders
participant SilverGenerators
participant ApplyCHMigrations
participant TaskMaterializedViews
SeedRunner->>CreateBronzePlaceholders: run create-bronze-placeholders.sh
CreateBronzePlaceholders-->>SeedRunner: create placeholders
SeedRunner->>SilverGenerators: seed silver rows
SilverGenerators-->>SeedRunner: populate silver tables
SeedRunner->>ApplyCHMigrations: run apply-ch-migrations.sh
ApplyCHMigrations-->>SeedRunner: build gold models and views
SeedRunner->>TaskMaterializedViews: refresh dependent views
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 |
- dev-compose.sh: print exposed service URLs (host ports from .env.compose) at the end of `up`, and add a standalone `urls` subcommand to reprint them. Local-only DBs and the frontend line are shown conditionally. - silver.py: rename apply_placeholders -> apply_create_bronze_placeholders to reflect the script it runs; comment/docstring tidy (run order unchanged: placeholders -> seed -> migrations+gold -> refresh task MVs). Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deploy/seed/generators/ai.py (1)
72-99: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLeave
conversation_countNULL for Cursor rowsdeploy/seed/generators/ai.py:72-99
Cursor maps this column toNULLin Silver, anddev_conversationssums it directly downstream. Setconversation_countonly for tools that report conversations; Cursor should writeNonehere.🤖 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 `@deploy/seed/generators/ai.py` around lines 72 - 99, Update the row construction in the _DEV_TOOLS loop so the conversation_count value is None when tool is Cursor, while retaining the existing sessions-based value for tools that report conversations. Keep all other generated metrics unchanged.
🤖 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 `@deploy/seed/silver.py`:
- Around line 51-66: Correct the host-run fallback in _ingestion_scripts_dir()
by resolving from the repository root rather than the deploy directory, so it
returns <repo>/src/ingestion/scripts. Keep the /ingestion/scripts
mounted-directory preference unchanged.
In `@dev-compose.sh`:
- Around line 403-412: Update the env_file assignment in the surrounding command
function to propagate a nonzero result from resolve_env_file immediately,
preserving its original error instead of continuing with an empty path. Remove
the frontend_up variable and the FRONTEND_MODE == "none" conditional, and call
report_service_urls using its default behavior.
---
Outside diff comments:
In `@deploy/seed/generators/ai.py`:
- Around line 72-99: Update the row construction in the _DEV_TOOLS loop so the
conversation_count value is None when tool is Cursor, while retaining the
existing sessions-based value for tools that report conversations. Keep all
other generated metrics unchanged.
🪄 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: 2f4f4e05-88eb-47f4-ab63-afb98f440b4e
📒 Files selected for processing (11)
deploy/compose/clickhouse-user-defaults.xmldeploy/seed/Dockerfiledeploy/seed/generators/ai.pydeploy/seed/generators/base.pydeploy/seed/generators/git.pydeploy/seed/generators/people.pydeploy/seed/generators/task.pydeploy/seed/requirements.txtdeploy/seed/silver.pydev-compose.shdocker-compose.yml
- silver.py: _ingestion_scripts_dir host fallback used .parent.parent (-> <repo>/deploy), so it resolved <repo>/deploy/src/ingestion/scripts and host runs failed. Use parents[2] (repo root). [CodeRabbit, major] - dev-compose.sh cmd_urls: propagate resolve_env_file failure with `|| return $?` (was swallowing it into an empty double-error), and drop the dead FRONTEND_MODE == "none" branch (cmd_up enforces dev|built|ghcr). [CodeRabbit, minor] Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
What
Rebuilds the sample-data seed so table + gold-layer setup uses the same mechanism as a real deployment instead of a Python reimplementation that skipped the dbt-owned gold layer.
silver.pynow runs the exact scripts the k8s clickhouse-migrate Hook Job runs:create-bronze-placeholders.sh— creates the silver tablesapply-ch-migrations.sh— migrations (gold views) + staging repair +dbt run --select tag:goldSeeding sits between steps 1 and 3 so the one
materialized='table'gold model (git_metric_observations) is built over real seeded rows rather than empty placeholders. Re-running the placeholder script inside step 3 is a no-op (IF NOT EXISTS), and the dbtdrop_silver_placeholders_at_starthook can't fire on atag:gold-only run, so seeded rows survive.Why
The old seed reimplemented DDL/migrations in Python and never built the 3 dbt-owned gold observation models — so
insight.ai_metric_observations,git_metric_observations, andmetric_entity_cohorts_currentdidn't exist on a seeded stack, and any metric routed through the unified observation runtime was blank.Changes
Mechanism
Dockerfile: pinpython:3.12-slim(matches the toolbox dbt image), addbash; adddbt-clickhousetorequirements.txt.docker-compose.yml: mount the full./src/ingestionat/ingestion(dbt needs the whole project), setCLICKHOUSE_DATABASE, redirect dbttarget//logs/to/tmp.clickhouse-user-defaults.xml: enableallow_experimental_refreshable_materialized_view— the migrationsSETit per-statement but the deploy runner (run_ch) is stateless; production CH allows it server-side.Data conformance (so the gold relations actually populate)
git.py: PRstate→MERGED/OPEN; addtenant_id+data_source; commitdata_source.ai.py: assistantsurface→chat; addconversation_count+ tool/surface display labels.people.py: seedworkspace_id(was collapsingmetric_entity_cohorts_currentto zero rows).task.py: seedclass_task_statusesdimension + carrystatus_idon status events (fixesjira_closed_tasks = 0).base.py:deterministic_intforInt64id columns (pr_id).Verification
Ran end-to-end against a local compose stack (CH 24.8):
Completed successfully, 5 gold tests PASS.metric_entity_cohorts_current: 25 rows / 5 cohorts (was 0).git_metric_observations:pr_merged270,pr_cycle_hours9728 (were 0/NULL); source dimensiongithub/GitHub.ai_metric_observations:dev_conversations4594,chat_assistant_conversations2110 (were 0); labels populated.jira_closed_tasks: 212 rows / 236 tasks closed (was 0).tenant_idconsistent across all three relations; observation↔cohort join resolves.Notes / out of scope
class_git_pull_requests_commitslink table left unseeded — it feeds a PR-author-email fallback that never fires (PRauthor_emailis always seeded).Summary by CodeRabbit
New Features
urlscommand to display local service addresses, including optional frontend, APIs, databases, and supporting services.Bug Fixes