You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Prepare a safe, explicit path for additive warehouse schema migrations.
Review Guide
Start with projects/onchain-analytics/scripts/deploy-warehouse.ps1.
Check that an unknown or historical SQL file cannot execute, the default is plan-only,
and production execution requires an explicit impersonated service account, both execution
switches, and a typed target confirmation.
Then review:
warehouse/L1/08_PipelineRunsOutcome_v1.sql: adds nullable columns without rewriting historical rows.
warehouse/L1/09_CreateRawLogs_v1.sql: creates the runtime raw-log shape with monthly partitions and a required partition filter.
warehouse/L1/11_CreateRawLogsAllHistory_v1.sql and 12_CreateTransactionsAllHistory_v1.sql: bounded read views, not replacements of existing objects.
scripts/ops/validate-l0-migrations.mjs: runs these files twice against guarded sandboxes, checks schema compatibility and synthetic legacy-row preservation, and verifies cleanup.
scripts/tests/deploy-warehouse.Tests.ps1: executable refusal tests for unlisted and historical SQL.
docs/03_OPERATIONS.md: approved deployment commands and stop conditions.
Paths above are relative to projects/onchain-analytics/ unless fully specified.
The 732-line, 14-file change is kept together because the allowlist, migrations, tests and
operator instructions describe one deployment contract.
Validation
Post-commit npm run verify:local: lint/typecheck passed; 543 default tests and 21 defect tests passed.
Statement coverage 80.19%; branch coverage 84.10%; function coverage 88.15%.
Deployment refusal tests, validator syntax and staged whitespace checks passed.
Publication leak scan: 14 files clean, no exceptions.
Recorded sandbox rehearsal applied all five migrations twice as single statements and
retained 22 synthetic PipelineRuns rows and 265 synthetic legacy reconciliation rows.
Production records were not copied into the rehearsal; their preservation must be checked
during actual commissioning. Cleanup was confirmed by dataset listing.
Not A Deployment Approval
No production DDL, ingestion, IAM grant or retention change is part of this publication.
The migrations are prepared, not applied. Review of this PR does not authorize executing them.
Commissioning still requires approval of the exact migration list, the restricted executor,
the administrator responsible for revocation, and the retention decision.
…oning
Replace folder-wide SQL deployment with a fixed five-migration allowlist and plan-only default. Production execution requires an explicit impersonated service account, an execution switch and target confirmation.
Create the raw-log schema and bounded read views, and extend bookkeeping schemas without replacing tables or inferring missing historical dimensions. Rehearse migrations against legacy-shaped sandbox fixtures, including idempotency and cleanup.
Review deploy-warehouse.ps1 first, then the five migration files and validate-l0-migrations.mjs. Deployment refusal tests and syntax checks pass; the staged leak scan is clean across all 14 files. Production execution, IAM changes and retention decisions are not included.
… nullability
Pass impersonation through the query child's CLOUDSDK_AUTH_IMPERSONATE_SERVICE_ACCOUNT environment instead of changing persistent gcloud configuration. Concurrent child processes retain distinct approved identities and leave the caller unchanged.
Parse NOT NULL from the runtime column definition rather than a nonexistent regex group. Add offline required/nullable parser regressions, overlapping-process identity checks and an installed-SDK environment check. Update operator guidance and local test commands.
The query measures old_rows_without_release_sha, but never checks it. These assertions would accept 22 fixture rows even if all their release hashes were populated, so the validator does not enforce the migration's historical-NULL guarantee for this field. Assert that all 22 fixture rows retain NULL release_sha values before reporting success.
Require all migrated historical fixture rows to retain NULL release_sha values. Offline regressions prove a populated hash is rejected even when row counts remain unchanged, while valid historical rows pass.
Fixed the remaining historical-NULL assertion in 72c737e. The validator now requires all 22 historical fixture rows to retain NULL release_sha. Offline regression controls reject both one populated hash and all populated hashes while unchanged historical rows pass (6/6 focused tests). Re-ran the actual guarded sandbox migration rehearsal on the new head: 17 capped jobs, 20 MiB billed, all schema and row-preservation assertions passed, sandbox cleanup proven by listing. All three CI checks pass and the full-range leak scan is CLEAN across 15 files with no exceptions. Final review requested for this head; merge is held until it returns with no unresolved findings. No production DDL or IAM changes were performed.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The production DDL execution safeguards warrant final human verification in the supported Windows SDK environment.
Review effort: Balanced Findings: None
Previously missed (1)
In code that hasn't changed since last review
Remove leading backslash from Windows deployment command
projects/onchain-analytics/README.md:171
Remove the extra leading backslash. On Windows, \.\scripts\deploy-warehouse.ps1 resolves from the current drive's root, not the project directory, so the documented plan-only command fails in a normal checkout. Use .\scripts\deploy-warehouse.ps1 instead.
Merged at c34c4c1 under explicit operator authorization. All three CI checks passed and the full-range leak scan was CLEAN across 15 files with no exceptions. Both implementation findings and the historical-NULL check were repaired with regression tests and sandbox validation; the final README path correction was exercised in plan-only mode. Copilot's last submitted review still covered the preceding commit and listed that corrected documentation issue; this merge does not claim a clean final-head Copilot review. No production migration, IAM grant, retention change or ingestion was performed. Production commissioning remains separately authorized.
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
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.
Prepare a safe, explicit path for additive warehouse schema migrations.
Review Guide
Start with
projects/onchain-analytics/scripts/deploy-warehouse.ps1.Check that an unknown or historical SQL file cannot execute, the default is plan-only,
and production execution requires an explicit impersonated service account, both execution
switches, and a typed target confirmation.
Then review:
warehouse/L1/08_PipelineRunsOutcome_v1.sql: adds nullable columns without rewriting historical rows.warehouse/L1/09_CreateRawLogs_v1.sql: creates the runtime raw-log shape with monthly partitions and a required partition filter.warehouse/L1/10_AddOracleReconciliationCompatibility_v1.sql: retains the legacy table_id; missing historical chain/address dimensions remain NULL.warehouse/L1/11_CreateRawLogsAllHistory_v1.sqland12_CreateTransactionsAllHistory_v1.sql: bounded read views, not replacements of existing objects.scripts/ops/validate-l0-migrations.mjs: runs these files twice against guarded sandboxes, checks schema compatibility and synthetic legacy-row preservation, and verifies cleanup.scripts/tests/deploy-warehouse.Tests.ps1: executable refusal tests for unlisted and historical SQL.docs/03_OPERATIONS.md: approved deployment commands and stop conditions.Paths above are relative to
projects/onchain-analytics/unless fully specified.The 732-line, 14-file change is kept together because the allowlist, migrations, tests and
operator instructions describe one deployment contract.
Validation
npm run verify:local: lint/typecheck passed; 543 default tests and 21 defect tests passed.retained 22 synthetic PipelineRuns rows and 265 synthetic legacy reconciliation rows.
Production records were not copied into the rehearsal; their preservation must be checked
during actual commissioning. Cleanup was confirmed by dataset listing.
Not A Deployment Approval
No production DDL, ingestion, IAM grant or retention change is part of this publication.
The migrations are prepared, not applied. Review of this PR does not authorize executing them.
Commissioning still requires approval of the exact migration list, the restricted executor,
the administrator responsible for revocation, and the retention decision.