fix(insights): stop implausible release years (e.g. year 400) at the source#382
Merged
Merged
Conversation
…source Genre Trends plotted releases at decades like 400/600/800 AD. Root cause: a Discogs *release* stores its date in `<released>` (a date string), not `<year>`, so the extractor's `nullify_when` / `year-out-of-range` rules — which key on the `year` field — were silent no-ops for releases. The release year is derived consumer-side in `_parse_year_int`, which only rejected year 0, so `released="0400-01-01"` was stored as year 400. - common/data_normalizer.py: `_parse_year_int` now bounds the parsed year to [MIN_RELEASE_YEAR (1860), current_year + 1], the single authoritative gate for both release and master years. Add `MIN_RELEASE_YEAR` constant; fix the misleading docstring that claimed the extractor nullifies release sentinels. - extractor/extraction-rules.yaml: document that the `year` filter is a no-op for releases (date lives in `released`), bounded consumer-side instead. - scripts/cleanup-implausible-years.sh: one-time cleanup (dry-run by default, `--apply` to act) that nulls out-of-range years in existing Neo4j and PostgreSQL data. Documented in scripts/README.md. - tests/common/test_data_normalizer.py: cover the new plausibility bounds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Move the primary year-plausibility filter to where bad data is supposed to be caught — the extractor's rules engine — instead of relying on the consumer. Two gaps prevented the existing rules from catching bad release dates: 1. A Discogs release has no `year` field; its date lives in `released` (a date string). The year-keyed rules never matched a release. 2. `nullify_when`'s range check did `parse::<f64>()` on the whole value, so a date string like "0400-01-01" failed to parse and was left untouched. - rules.rs: `nullify_when` range now falls back to the leading year component (`parse_leading_year`) when the value isn't a plain number, so date fields like `released` are range-checked. - extraction-rules.yaml: add a `released` nullify_when filter for releases (below 1860, above 2027) and add the `above` upper bound to the existing `year` filters (releases + masters) for parity. - rules_tests.rs: cover date-string range filtering (antiquity, far-future, plausible full/partial dates). The consumer-side bound in common/data_normalizer.py remains as a defensive backstop (see prior commit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
E2E Coverage (webkit)
|
Contributor
E2E Coverage (chromium)
|
Contributor
E2E Coverage (firefox)
|
Contributor
E2E Coverage (webkit - iPhone 15)
|
Contributor
E2E Coverage (webkit - iPad Pro 11)
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
The Insights Genre Trends chart plotted releases at decades like 400 / 600 / 800 AD, stretching the x-axis back to antiquity.
This was not stale pre-filter data — the host was provisioned after the year filter shipped, so every ingest ran with the filter active. The filter simply never applied to releases.
Root cause (two gaps, both in the extractor)
A Discogs release stores its date in
<released>(a date string like"1969-09-26"), not<year>:nullify_when/year-out-of-rangerules key on theyearfield, which doesn't exist on a release — so they were silent no-ops for releases (masters, which have<year>, were fine).released,nullify_when's range check didparse::<f64>()on the whole value, so"0400-01-01"failed to parse and was left untouched.The release year was then derived consumer-side in
common/data_normalizer.py→_parse_year_int(data.get("released")), which only rejectedyear == 0. So"0400-01-01"→400→ written straight to Neo4j.Fix — at the rules engine (where bad data is meant to be caught), plus a defensive backstop and cleanup
Extractor (primary):
extractor/src/rules.rs—nullify_when's range check now falls back to the leading year component (parse_leading_year) when the value isn't a plain number, so date fields likereleasedare range-checked.extractor/extraction-rules.yaml— add areleasednullify_whenfilter for releases (below: 1860,above: 2027), and add theaboveupper bound to the existingyearfilters (releases + masters) for parity. Bad release dates are now nullified at the extractor, before reaching any consumer.Consumer (defensive backstop):
common/data_normalizer.py—_parse_year_intbounds the parsed year to[MIN_RELEASE_YEAR (1860), current_year + 1](addedMIN_RELEASE_YEAR; corrected the misleading docstring). Catches anything that slips past, for both releases and masters.Existing data:
scripts/cleanup-implausible-years.sh— one-time cleanup for rows already ingested via the buggy path. Dry-run by default;--applynulls out-of-range years in existing Neo4j (Release/Master) and PostgreSQL (releases/mastersJSONB) records. Documented inscripts/README.md.Verification
cargo test(extractor) — all pass incl. newtest_nullify_when_released_date_string_year_out_of_range;cargo clippy -D warnings+cargo fmt --checkcleanuv run pytest tests/common tests/graphinator tests/tableinator— all pass;ruff+mypycleanshellcheck+bash -non the cleanup script — cleanOperator note
After merge, run on the host once to clean existing data:
Scope
MusicBrainz entities take a different ingest path (no
normalize_record, noyearwrite) and are out of scope.🤖 Generated with Claude Code