[joelridden]

Why

SeismicNow is getting physics-based simulations of recent NZ earthquakes, served from simulation_db's new realtime dataset. Each new event's results arrive as their own single-event imdb file (plus a waveform .nc), and simulation_db has to add that event to the one growing realtime.duckdb without rebuilding it. Re-running a simulation has to replace the event cleanly. imdb had no way to copy events between databases, so this adds one.

What

IMDB.merge_from(other, replace=True) copies every event in other into this database: events, realisations, site-event distances, records and their IMs (scalars, pSA, FAS and sigma).

• Sites are matched by site_id. New sites are added and existing ones are updated with the incoming values.
• Re-ingest: with replace=True, an existing event is deleted first, so the latest upload replaces it. With replace=False, an existing event is an error.
• Safety checks: the period and frequency grids must match exactly, and the incoming components must all exist here. Otherwise it raises ValueError.
• Exact copies: records are inserted in groups by which IM rows they have, so there are no empty scalar rows and no sigma arrays on simulated records. This was checked by merging the alpine dataset into an empty copy.
• Not atomic, because add_records commits per call. Callers should merge into a copy and swap it in once validate() passes, which is what simulation_db's ingest does.

New private helpers: _grid, _ims_by_record and _columns. There's no schema change, so SCHEMA_VERSION stays at 2.

Tests

tests/test_merge.py has 7 tests: copying, replacing, site updates, grid and component mismatches, and exact round-trips. The full suite passes. simulation_db's realtime ingest tests (239 in total) also pass against this code.

🤖 Generated with Claude Code

[joelridden]

@claude review

[claude]

Claude finished @joelridden's task in 5m 57s —— View job

---

Review complete

• Read repo guidelines (no CLAUDE.md / REVIEW.md present in this repo)
• Check existing review comments (none found)
• Read imdb/imdb.py diff (merge_from, _grid, _ims_by_record, _columns)
• Read tests/test_merge.py
• Post findings

Findings: 1 (medium severity, confirmed)

Posted inline on imdb/imdb.py (lines 645–663): the shared-site update statement (UPDATE sites SET ... FROM _merge_sites) overwrites every non-key column unconditionally from the incoming row, with no NULL guard. A site shared between the merge source and target gets its vs30/z1p0/z2p5/is_real/metadata nulled out if the incoming single-event file lacks that metadata, even though that site may be referenced by many unrelated events already in the target. This is exactly the shape of input the PR's motivating realtime-ingest use case would produce (a per-event file that may not carry full site characterisation), so it's a credible, repeatable metadata-loss path. The existing test only covers the NULL→value direction, not the regression direction.

Everything else held up well under review: the grid/component compatibility checks, re-ingest/delete-then-insert ordering, the id-mapping between source and target int surrogates, and the record-grouping logic that keeps scalar/pSA/FAS/sigma copies exact (verified against add_records' NaN-row-exclusion and rotd-nulling behaviour) are all internally consistent and correctly ordered. The documented non-atomicity is an accepted, disclosed limitation, not a new defect.

[joelridden]

@claude review

[claude]

Claude finished @joelridden's task in 2m 44s —— View job

---

Review complete

• Read repo guidelines (no CLAUDE.md / REVIEW.md present)
• Read existing review comments — the earlier finding (shared-site UPDATE blindly overwriting good data with incoming NULLs) was fixed in a994ba8 via COALESCE(m.{c}, sites.{c}), with a new regression test (test_merge_keeps_site_values_missing_from_incoming). Confirmed fixed, not re-reporting.
• Read git diff origin/main...HEAD (imdb/imdb.py, tests/test_merge.py)
• Review merge_from, _grid, _ims_by_record, _columns for new/newly-broken issues
• Review tests/test_merge.py
• Post findings

Findings: 0

No new correctness bugs found. Specifically checked and confirmed correct:

• The _ims_by_record refactor (a70d8f9) that replaced .iterrows()/.groupby() with positional numpy indexing — row order between rows.index, rows[column], and rows[f"{column}_sigma"] stays aligned through the zip, and the record-grouping via np.unique(groups, axis=0) / np.flatnonzero preserves the same per-record positional correspondence as the original groupby version.
• Ordering of operations in merge_from: existing events are deleted (cascading through records/site_event/realisations) before sites are updated and events/realisations/site_event/records are re-added, so there's no FK or duplicate-key collision on re-ingest.
• The site-merge COALESCE fix is complete: it only guards against incoming NULLs, still lets incoming non-NULL values (including explicit False) win, matching the documented "incoming values take priority, but a NULL doesn't erase a known value" semantics, and the new test exercises the NULL-value and still-wins-on-non-NULL cases together.
• add_records's own duplicate/collision checks (rel_id/site_id/component/kind/gmm_key) and NaN-row IM exclusion are unaffected by the merge path and still apply per group.
• Grid/component compatibility checks correctly handle the no-FAS-on-either-side edge cases (len(frequencies) guard before comparing).

[lispandfound]

Claude point is relevant. Otherwise looks fine.