curation: rank measurements_latest by the episode's own recorded_at - #52
Conversation
A repair pass that wins the episodes create-if-absent race can still crash before reaching the measurements table; a later retry early-returns on exists(episodes) and never revisits it, leaving that table's recorded_at stale forever (Hebbian-Robotics#51). measurements_latest ranked by its own recorded_at column, so it could then disagree with episodes_latest about which run is newest for the same episode. Join to episodes_raw and rank (and report) off its recorded_at instead -- the one column create-if-absent guarantees a single writer for.
kstonekuan
left a comment
There was a problem hiding this comment.
Thank you @chintondutta. Another self-found correctness bug with a precise writeup, a fix that targets the actual invariant, and a regression test that locks it in. That is three for three.
What I validated locally:
- Full quality gate is clean (
ruff check,ruff format --check,ty check) and all 307 tests pass, including the newtest_measurements_latest_ranks_by_the_owning_episodes_recorded_at. - Reproduced your pre-fix failure by checking main's
curation.pyinto the branch: the test picks the stale 1.0 exactly as you described, and passes with the fix. - Confirmed the wide
episodesview builds its measurement columns frommeasurements_latest, so everyday queries inherit the correction too.
What I especially like: ranking off the one column that create-if-absent guarantees a single writer for, instead of trying to catch every crash window at write time. The inner join dropping runs whose episodes file has not landed is the right call and matches append_episode's own "episodes existing proves the append completed" idiom. The comment in the view explaining all of that will save the next reader a lot of archaeology.
Merging now.
One ask before your next batch: please stick to one open pull request at a time, and leave the good first issues for newcomers. The bot closed the seven you opened yesterday for that reason, and none of the work is lost, but opening them all at once crowds out first-time contributors and outruns our review bandwidth. Honestly, starter issues are beneath what you are doing here. Bugs like #44, #46, and #51 are worth far more to the project, and if you want a fresh hunting ground, try running HFlow against a real corpus like Egocentric-10K and telling us what breaks, what is slow, or what is awkward. Come say hi on Discord if you have not already.
Fixes #51.
The gap
#47 made
episodes/<file_stem>.parquetthe single source of truth forrecorded_aton a given append -- but only for theepisodestable itself.measurements_lateststill ranks rows by its ownrecorded_atcolumn:A repair pass that wins the
episodesrace can still crash before reachingmeasurements(Catalog.append_episode's repair loop insrc/hflow/catalog.py). Becauseexists(episodes/...)is the only thing a later retry checks before early-returning, that dependent'srecorded_atis never revisited -- it can be permanently older or newer than the episode it belongs to.measurements_latestandepisodes_latestcan then disagree about which run is "latest" for the sameepisode_id, reproducing the class of bug #47 fixed, just from a narrower crash window instead of a live race.Fix
Join
measurementstoepisodes_rawon(episode_id, run_fingerprint)and rank -- and report --recorded_atfrom the episode side, the one columncreate-if-absentguarantees a single writer for.episodes_latestitself needed no change (it already IS the authoritative source). The inner join also means a run whoseepisodesfile hasn't landed yet (an append still mid-flight or crashed before completing) is invisible inmeasurements_latest, matchingappend_episode's own idiom thatepisodesexisting is what proves an append complete.Testing
New regression test
test_measurements_latest_ranks_by_the_owning_episodes_recorded_at(tests/test_catalog_curation.py): records an older and a newer run of the same episode, then corrupts the newer run'smeasurementsfile to carry an olderrecorded_atthan the older run's (simulating the crash-mid-repair window). Confirmed it fails against pre-fix code (picks the stale 1.0 instead of 2.0) and passes with the fix.No stored-format changes -- this only changes which
recorded_atameasurements_latestrow reports and how it's ranked, never what's stored on disk.