Skip to content

feat(db): normalise series index into shared index_axis table#747

Merged
lewisjared merged 5 commits into
mainfrom
feat/normalise-series-index
Jun 19, 2026
Merged

feat(db): normalise series index into shared index_axis table#747
lewisjared merged 5 commits into
mainfrom
feat/normalise-series-index

Conversation

@lewisjared

@lewisjared lewisjared commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Description

Series metric values previously stored their full index array (and index name) inline on every row. In production a handful of axes — for example a single common monthly time axis — are shared by tens of thousands of series, so the index column dominated the database: it accounted for ~40% of a 922MB production database and was almost entirely redundant.

This moves the index into a deduplicated index_axis table keyed by a content hash (sha256 over (name, values)), and replaces metric_value.index / metric_value.index_name with an index_id foreign key. Ingestion (ingest_series_values) now resolves the distinct axes for a batch once via SeriesIndex.get_or_create and references them by id.

SeriesMetricValue.index and SeriesMetricValue.index_name remain available as read-only properties resolved from the shared axis (loaded eagerly via lazy="joined"), so existing readers — including the ref-app API and the series length validator — are unaffected. The on-disk series.json shape (the climate_ref_core Pydantic type) is unchanged.

The Alembic migration is lossless: it streams the series rows, deduplicates axes by content hash, backfills index_id in batches, then drops the old columns. Verified against a copy of the production database it reclaimed 388MB (42%, 924MB -> 536MB) with zero hash collisions and zero reconstruction mismatches across the sampled rows. Note that a VACUUM is required after applying the migration for SQLite to release the freed pages on disk.

Checklist

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Changelog item added to changelog/

Summary by CodeRabbit

  • Database Optimisation

    • Series metric index axes are now deduplicated into a shared structure, with series records referencing the shared axis to reduce storage overhead and improve ingestion efficiency.
    • SQLite users should run VACUUM after the upgrade to reclaim freed disk space.
  • Chores

    • Improved CI coverage notifications to wait for all coverage uploads before posting status checks and aligning related comments.

Series metric values previously stored their full index array and index name
inline on every row.
In production a handful of axes (e.g. a common monthly time axis)
are shared by tens of thousands of series,
so the index dominated the database
(~40% of a 922MB production database, almost entirely redundant).

Move the index into a deduplicated `index_axis` table keyed by a content hash,
and replace `metric_value.index` / `index_name` with an `index_id` foreign key.
`SeriesMetricValue.index` and `index_name` remain available
as read-only properties resolved from the shared axis,
so existing readers (including the ref-app API) are unaffected.

The migration is lossless;
verified against a copy of the production database
it reclaimed 388MB (42%),
with no hash collisions and no reconstruction mismatches.
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.36364% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...te-ref/src/climate_ref/executor/result_handling.py 87.50% 0 Missing and 1 partial ⚠️
...climate-ref/src/climate_ref/models/metric_value.py 97.82% 1 Missing ⚠️
Flag Coverage Δ
core 92.59% <96.36%> (+0.03%) ⬆️
providers 91.82% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ges/climate-ref/src/climate_ref/models/__init__.py 100.00% <100.00%> (ø)
...te-ref/src/climate_ref/executor/result_handling.py 94.64% <87.50%> (-0.60%) ⬇️
...climate-ref/src/climate_ref/models/metric_value.py 93.25% <97.82%> (+2.69%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

`index` is not in SQLAlchemy's dialect reserved-word tables,
so it was emitted unquoted and PostgreSQL rejected `DROP COLUMN index`.
Force the quote via `quoted_name` so the DDL is valid on both PostgreSQL and SQLite.
The unit suite only exercises SQLite, which recreates the table in batch mode
and so never hit the bare identifier.
Set `notify.after_n_builds` (and `comment.after_n_builds`) to 6 so codecov
waits for every coverage report before posting any status, instead of marking
`codecov/project` failed against partial coverage while jobs are still running.
Count = tests-core (1) + tests-integration (1) + tests-providers (4).
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 922126b7-94a0-4e45-a672-f021f4b9a7b0

📥 Commits

Reviewing files that changed from the base of the PR and between 7e8dc06 and 30d17b0.

📒 Files selected for processing (1)
  • packages/climate-ref/src/climate_ref/migrations/versions/2026-06-19T0000_d4e5f6a7b8c9_normalise_series_index.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/climate-ref/src/climate_ref/migrations/versions/2026-06-19T0000_d4e5f6a7b8c9_normalise_series_index.py

📝 Walkthrough

Walkthrough

Introduces a SeriesIndex ORM model and index_axis database table to deduplicate series metric value index axes across rows. SeriesMetricValue is refactored to reference a shared axis via a foreign key and expose index metadata as computed properties. An Alembic migration backfills existing data by streaming rows, computing content-addressed hashes, and deduplicating axes in-memory before batch-updating foreign keys. The ingestion path in result_handling.py is updated to resolve axes via get_or_create. Codecov notification timing configuration is also added.

Changes

SeriesIndex Normalisation

Layer / File(s) Summary
SeriesIndex model definition
packages/climate-ref/src/climate_ref/models/metric_value.py
Expands imports to include hashlib, json, and Sequence. Introduces SeriesIndex SQLAlchemy model with id, hash (unique indexed), name, values, and length fields. Implements compute_hash static method to generate deterministic SHA-256 content addresses and get_or_create class method to fetch or insert axes by hash.
SeriesMetricValue schema refactor and API updates
packages/climate-ref/src/climate_ref/models/metric_value.py
Refactors SeriesMetricValue to store optional index_id foreign key and joined index_axis relationship, replacing direct stored "index" and index_name columns with computed @property accessors. Updates build classmethod to accept index_axis: SeriesIndex instead of raw index/index_name parameters. Updates before_insert/before_update validators to enforce len(values) consistency using target.index_axis.length.
Model package exports and unit test updates
packages/climate-ref/src/climate_ref/models/__init__.py, packages/climate-ref/tests/unit/models/test_metric_value.py
Updates models/__init__.py to import and export SeriesIndex. Updates unit tests to construct index axes via SeriesIndex.get_or_create(...) and pass index_axis=axis to SeriesMetricValue.build and mismatch validation tests.
Alembic migration: normalise_series_index
packages/climate-ref/src/climate_ref/migrations/versions/2026-06-19T0000_d4e5f6a7b8c9_normalise_series_index.py
Creates index_axis table with unique hash index, adds metric_value.index_id foreign key and index. Backfills by streaming SERIES rows, computing SHA-256 hashes per (index_name, values) tuple, deduplicating axes in-memory, inserting distinct axes, and batch-updating metric_value.index_id. Drops old inline "index" and index_name columns. Includes downgrade path that restores inline columns and repopulates them from index_axis via index_id lookups.
Ingestion path update and changelog
packages/climate-ref/src/climate_ref/executor/result_handling.py, changelog/747.improvement.md
Updates ingest_series_values to resolve shared index axes via SeriesIndex.get_or_create before constructing SeriesMetricValue rows, passing the resolved axis instead of inline index fields. Adds changelog entry documenting the deduplication into index_axis and SQLite VACUUM guidance.

Codecov Configuration

Layer / File(s) Summary
Codecov notify and comment timing
codecov.yml
Adds notify.wait_for_ci: true, notify.after_n_builds: 6, and comment.after_n_builds: 6 to delay status checks and PR comments until all six coverage uploads complete.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main database normalization change: moving series index into a shared, deduplicated index_axis table.
Description check ✅ Passed The description provides detailed context, technical implementation details, performance impact, and checklist confirmation. Tests and changelog items have been confirmed as added.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/normalise-series-index

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/climate-ref/src/climate_ref/executor/result_handling.py (1)

180-201: 💤 Low value

Minor: Double hash computation per series.

The hash is computed twice for each series_result — once in the first loop (line 185) and once in the second loop (line 192). This is not incorrect, but you could store the digest alongside the series data in the first pass to avoid recomputing it.

♻️ Suggested optimisation
     axis_id_by_hash: dict[str, int] = {}
-    for series_result in series_values:
-        digest = SeriesIndex.compute_hash(series_result.index_name, series_result.index)
+    series_with_digest: list[tuple[str, Any]] = []
+    for series_result in series_values:
+        digest = SeriesIndex.compute_hash(series_result.index_name, series_result.index)
+        series_with_digest.append((digest, series_result))
         if digest not in axis_id_by_hash:
             axis = SeriesIndex.get_or_create(database.session, series_result.index_name, series_result.index)
             axis_id_by_hash[digest] = axis.id

     new_values = []
-    for series_result in series_values:
-        digest = SeriesIndex.compute_hash(series_result.index_name, series_result.index)
+    for digest, series_result in series_with_digest:
         new_values.append(
             {
                 "execution_id": execution.id,

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a9823cbe-f78f-4252-966a-426a9d4a18e6

📥 Commits

Reviewing files that changed from the base of the PR and between 82aff55 and 7e8dc06.

📒 Files selected for processing (7)
  • changelog/747.improvement.md
  • codecov.yml
  • packages/climate-ref/src/climate_ref/executor/result_handling.py
  • packages/climate-ref/src/climate_ref/migrations/versions/2026-06-19T0000_d4e5f6a7b8c9_normalise_series_index.py
  • packages/climate-ref/src/climate_ref/models/__init__.py
  • packages/climate-ref/src/climate_ref/models/metric_value.py
  • packages/climate-ref/tests/unit/models/test_metric_value.py

…migration

Setting `stream_results=True` via `bind.execution_options(...)` mutates the shared
migration connection, so the option persisted to the subsequent DDL.
On PostgreSQL that wraps the `DROP COLUMN` in a server-side cursor
(`DECLARE ... CURSOR WITHOUT HOLD FOR ALTER TABLE ...`), which is a syntax error.
Apply the option to the SELECT statement only.
Verified end-to-end against PostgreSQL 16; SQLite was unaffected (no server-side
cursors), which is why the unit suite missed it.
@lewisjared lewisjared merged commit 36d9cbd into main Jun 19, 2026
26 checks passed
@lewisjared lewisjared deleted the feat/normalise-series-index branch June 19, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant