Skip to content

Commit 8d2a32d

Browse files
fix(insights): stop implausible release years (e.g. year 400) at the source (#382)
* fix(insights): stop implausible release years (e.g. year 400) at the 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> * fix(extractor): filter implausible release dates at the rules engine 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> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent dfb0c4a commit 8d2a32d

7 files changed

Lines changed: 307 additions & 17 deletions

File tree

β€Žcommon/data_normalizer.pyβ€Ž

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,38 +6,57 @@
66
point that consumers call.
77
"""
88

9+
from datetime import UTC, datetime
910
from typing import Any
1011

1112
import structlog
1213

1314

1415
logger = structlog.get_logger(__name__)
1516

17+
#: Earliest plausible release year. Sound recording predates this only
18+
#: marginally (Scott de Martinville's phonautograms, 1857), so anything
19+
#: earlier is a data error β€” typos, sentinels, or mis-parsed dates.
20+
#: Must stay in sync with the year bounds in ``extractor/extraction-rules.yaml``.
21+
MIN_RELEASE_YEAR = 1860
22+
23+
24+
def _max_release_year() -> int:
25+
"""Latest plausible release year: next calendar year (allows pre-orders/announcements)."""
26+
return datetime.now(UTC).year + 1
27+
1628

1729
def _parse_year_int(value: Any) -> int | None:
18-
"""Parse a Discogs year value into an integer.
30+
"""Parse a Discogs year value into a *plausible* integer year.
1931
2032
Handles both plain year strings ("1969", as used in Master.<year>) and
2133
full/partial date strings ("1969-09-26", "1969-00-00", as used in
22-
Release.<released>). Returns None when no valid year is found.
34+
Release.<released>). Returns None when no valid year is found **or** when
35+
the year falls outside ``[MIN_RELEASE_YEAR, current_year + 1]``.
2336
24-
Note: The extractor's ``nullify_when`` filter now converts sentinel years
25-
(year < 1860, including 0) to null before messages reach consumers. The
26-
``year == 0`` check below is a defensive fallback for extractions run
27-
without the updated rules config.
37+
This is the authoritative year gate for releases. The extractor's
38+
``nullify_when`` / ``year-out-of-range`` rules key on a record's ``year``
39+
field, but a Discogs *release* carries its date in ``released`` (a date
40+
string) and has no ``year`` field at extraction time β€” so those rules are
41+
no-ops for releases. Release (and master) years are derived here, so the
42+
plausibility bound must be enforced here too; otherwise dates like
43+
``"0400-01-01"`` leak into the graph as year 400.
2844
"""
2945
if value is None:
3046
return None
3147
if isinstance(value, int):
32-
return value if value != 0 else None
33-
s = str(value).strip()
34-
if not s:
35-
return None
36-
try:
37-
year = int(s[:4])
38-
return year if year != 0 else None
39-
except ValueError:
40-
return None
48+
year = value
49+
else:
50+
s = str(value).strip()
51+
if not s:
52+
return None
53+
try:
54+
year = int(s[:4])
55+
except ValueError:
56+
return None
57+
if MIN_RELEASE_YEAR <= year <= _max_release_year():
58+
return year
59+
return None
4160

4261

4362
def normalize_record(data_type: str, data: dict[str, Any]) -> dict[str, Any]:

β€Žextractor/extraction-rules.yamlβ€Ž

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,24 @@ filters:
2727
- field: genres.genre
2828
remove_matching: "^\\d+$"
2929
reason: "Strip legacy numeric genre IDs from upstream data"
30+
# A Discogs *release* has no `year` field at extraction β€” its date lives in
31+
# `released` (a date string like "1969-09-26"). The nullify_when range filter
32+
# parses the leading year out of such date strings, so an implausible date
33+
# like "0400-01-01" is nullified here, at the extractor, before it reaches
34+
# any consumer. (data_normalizer._parse_year_int re-checks the bound as a
35+
# defensive backstop.)
36+
- field: released
37+
nullify_when:
38+
type: range
39+
below: 1860
40+
above: 2027
41+
reason: "Treat sentinel and implausible release dates as unknown"
42+
# Kept for robustness in case upstream ever emits a release `year` field.
3043
- field: year
3144
nullify_when:
3245
type: range
3346
below: 1860
47+
above: 2027
3448
reason: "Treat sentinel and implausible years as unknown"
3549
masters:
3650
- field: genres.genre
@@ -40,6 +54,7 @@ filters:
4054
nullify_when:
4155
type: range
4256
below: 1860
57+
above: 2027
4358
reason: "Treat sentinel and implausible years as unknown"
4459

4560
rules:

β€Žextractor/src/rules.rsβ€Ž

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,22 @@ pub struct FilterAction {
385385
pub reason: String,
386386
}
387387

388+
/// Extract a numeric year from a date-like string for range comparison.
389+
///
390+
/// `nullify_when` range filters compare numerically, but a Discogs *release*
391+
/// stores its date in `released` as a string like `"1969-09-26"` (or the
392+
/// implausible `"0400-01-01"`). Such values don't parse as a single number, so
393+
/// callers fall back to this β€” the leading date component (the year) β€” so the
394+
/// year-range filter can catch bad release dates the same way it catches bad
395+
/// master years.
396+
fn parse_leading_year(value: &str) -> Option<f64> {
397+
let token = value.split(['-', '/', '.', ' ', 'T']).next()?.trim();
398+
if token.is_empty() {
399+
return None;
400+
}
401+
token.parse::<f64>().ok()
402+
}
403+
388404
pub fn apply_filters(config: &CompiledRulesConfig, data_type: &str, record: &mut Value) -> Vec<FilterAction> {
389405
let conditions = config.filters_for(data_type);
390406
let mut actions = Vec::new();
@@ -450,14 +466,18 @@ pub fn apply_filters(config: &CompiledRulesConfig, data_type: &str, record: &mut
450466
Value::Null => continue, // already null β€” no action
451467
_ => continue,
452468
};
453-
if let Ok(num) = str_val.parse::<f64>() {
469+
// Compare numerically. A plain number ("1969") parses directly;
470+
// a date string ("1969-09-26", "0400-01-01") falls back to its
471+
// leading year component so date fields like `released` can be
472+
// range-checked too.
473+
if let Some(num) = str_val.parse::<f64>().ok().or_else(|| parse_leading_year(&str_val)) {
454474
let should_nullify = below.is_some_and(|b| num < b) || above.is_some_and(|a| num > a);
455475
if should_nullify {
456476
obj.insert(field.clone(), Value::Null);
457477
actions.push(FilterAction { field: field.clone(), removed_count: 1, removed_values: vec![str_val], reason: reason.clone() });
458478
}
459479
}
460-
// Non-numeric string: leave unchanged
480+
// Non-numeric, non-date string: leave unchanged
461481
}
462482
}
463483
}

β€Žextractor/src/tests/rules_tests.rsβ€Ž

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,6 +1546,51 @@ rules: {}
15461546
assert!(actions2.is_empty());
15471547
}
15481548

1549+
#[test]
1550+
fn test_nullify_when_released_date_string_year_out_of_range() {
1551+
// A Discogs release stores its date in `released` (a date string), not `year`.
1552+
// The range filter must parse the leading year out of the date so implausible
1553+
// release dates are nullified at the extractor.
1554+
let config = compile_yaml(
1555+
r#"
1556+
filters:
1557+
releases:
1558+
- field: released
1559+
nullify_when:
1560+
type: range
1561+
below: 1860
1562+
above: 2027
1563+
reason: "Implausible release date"
1564+
rules: {}
1565+
"#,
1566+
);
1567+
1568+
// Antiquity date -> null (this is the bug: "0400-01-01" was stored as year 400)
1569+
let mut record = json!({"released": "0400-01-01", "title": "Test"});
1570+
let actions = apply_filters(&config, "releases", &mut record);
1571+
assert_eq!(record["released"], json!(null));
1572+
assert_eq!(actions.len(), 1);
1573+
assert_eq!(actions[0].removed_values, vec!["0400-01-01"]);
1574+
1575+
// Far-future date -> null
1576+
let mut record2 = json!({"released": "9999-12-31", "title": "Test"});
1577+
let actions2 = apply_filters(&config, "releases", &mut record2);
1578+
assert_eq!(record2["released"], json!(null));
1579+
assert_eq!(actions2.len(), 1);
1580+
1581+
// Plausible full date -> preserved
1582+
let mut record3 = json!({"released": "1969-09-26", "title": "Test"});
1583+
let actions3 = apply_filters(&config, "releases", &mut record3);
1584+
assert_eq!(record3["released"], json!("1969-09-26"));
1585+
assert!(actions3.is_empty());
1586+
1587+
// Partial date (year only, zeroed month/day) -> preserved
1588+
let mut record4 = json!({"released": "1987-00-00", "title": "Test"});
1589+
let actions4 = apply_filters(&config, "releases", &mut record4);
1590+
assert_eq!(record4["released"], json!("1987-00-00"));
1591+
assert!(actions4.is_empty());
1592+
}
1593+
15491594
#[test]
15501595
fn test_nullify_when_non_numeric_unchanged() {
15511596
let config = compile_yaml(

β€Žscripts/README.mdβ€Ž

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,33 @@ NEO4J_CONTAINER=my-neo4j NEO4J_PASSWORD=secret ./scripts/migrate-master-year-to-
182182
- `NEO4J_USER`: Neo4j username (default: `neo4j`)
183183
- `NEO4J_PASSWORD`: Neo4j password (default: `discogsography`)
184184

185+
## πŸ“… cleanup-implausible-years.sh
186+
187+
One-time cleanup that nulls out implausible release/master years (outside `[1860, current_year + 1]`) from existing data in **both** Neo4j and PostgreSQL.
188+
189+
Discogs *releases* carry their date in `<released>` (a date string), not `<year>`, so the extractor's year-range rules β€” which key on the `year` field β€” never fired for releases. Before the `common/data_normalizer.py` fix, a release dated `"0400-01-01"` was stored as year `400`, polluting the Insights "Genre Trends" chart. The code fix stops new bad years at ingest; this script aligns historical data without a full re-ingest. MusicBrainz entities use a different ingest path and are out of scope.
190+
191+
The script defaults to a **dry run** (counts only). Pass `--apply` to make changes.
192+
193+
### Usage
194+
195+
```bash
196+
# Dry run β€” report how many records would be affected, no changes
197+
./scripts/cleanup-implausible-years.sh
198+
199+
# Perform the cleanup
200+
./scripts/cleanup-implausible-years.sh --apply
201+
```
202+
203+
### Environment Variables
204+
205+
- `NEO4J_CONTAINER`: Neo4j container name (default: `discogsography-neo4j`)
206+
- `NEO4J_USER`: Neo4j username (default: `neo4j`)
207+
- `NEO4J_PASSWORD`: Neo4j password (default: `discogsography`)
208+
- `POSTGRES_CONTAINER`: PostgreSQL container name (default: `discogsography-postgres`)
209+
- `POSTGRES_USER`: PostgreSQL username (default: `discogsography`)
210+
- `POSTGRES_DB`: PostgreSQL database (default: `discogsography`)
211+
185212
## 🐳 neo4j-entrypoint.sh
186213

187214
Thin wrapper for the Neo4j Docker container entrypoint. Reads the password from `/run/secrets/neo4j_password` and sets `NEO4J_AUTH` before delegating to the official Neo4j entrypoint. This is needed because Neo4j does not natively support the Docker `_FILE` secret convention.
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
#!/usr/bin/env bash
2+
# cleanup-implausible-years.sh
3+
#
4+
# One-time cleanup: nulls out implausible release/master years from existing
5+
# data in Neo4j and PostgreSQL.
6+
#
7+
# Background: a Discogs *release* carries its date in <released> (a date string),
8+
# not <year>, so the extractor's year-range rules β€” which key on the `year`
9+
# field β€” never fired for releases. The release year is derived consumer-side
10+
# in common/data_normalizer.py, which (prior to this fix) only rejected year 0.
11+
# As a result, releases with antiquity/sentinel dates (e.g. "0400-01-01") were
12+
# stored as year 400, polluting the Insights "Genre Trends" chart.
13+
#
14+
# The code fix (common/data_normalizer.py) stops new bad years at ingest. This
15+
# script brings *existing* data in line without a full re-ingest. New ingests
16+
# need no cleanup.
17+
#
18+
# Scope: Discogs Release/Master nodes (Neo4j) and the public `releases`/`masters`
19+
# tables (PostgreSQL). MusicBrainz entities take a different ingest path and are
20+
# out of scope.
21+
#
22+
# Plausible range: [1860, current_year + 1] β€” must stay in sync with
23+
# MIN_RELEASE_YEAR in common/data_normalizer.py and extractor/extraction-rules.yaml.
24+
#
25+
# Usage:
26+
# ./scripts/cleanup-implausible-years.sh # dry run β€” counts only, no changes
27+
# ./scripts/cleanup-implausible-years.sh --apply # perform the cleanup
28+
#
29+
# Environment variables (all optional, defaults shown):
30+
# NEO4J_CONTAINER docker container name (default: discogsography-neo4j)
31+
# NEO4J_USER Neo4j username (default: neo4j)
32+
# NEO4J_PASSWORD Neo4j password (default: discogsography)
33+
# POSTGRES_CONTAINER docker container name (default: discogsography-postgres)
34+
# POSTGRES_USER PostgreSQL username (default: discogsography)
35+
# POSTGRES_DB PostgreSQL database (default: discogsography)
36+
37+
set -euo pipefail
38+
39+
NEO4J_CONTAINER="${NEO4J_CONTAINER:-discogsography-neo4j}"
40+
NEO4J_USER="${NEO4J_USER:-neo4j}"
41+
NEO4J_PASSWORD="${NEO4J_PASSWORD:-discogsography}"
42+
POSTGRES_CONTAINER="${POSTGRES_CONTAINER:-discogsography-postgres}"
43+
POSTGRES_USER="${POSTGRES_USER:-discogsography}"
44+
POSTGRES_DB="${POSTGRES_DB:-discogsography}"
45+
46+
MIN_YEAR=1860
47+
MAX_YEAR=$(($(date +%Y) + 1))
48+
49+
APPLY=0
50+
if [[ "${1:-}" == "--apply" ]]; then
51+
APPLY=1
52+
elif [[ -n "${1:-}" ]]; then
53+
echo "❌ Unknown argument '${1}'. Use --apply to perform the cleanup (omit for a dry run)."
54+
exit 1
55+
fi
56+
57+
if [[ "${APPLY}" -eq 1 ]]; then
58+
echo "⚠️ APPLY mode: implausible years will be set to null."
59+
else
60+
echo "πŸ§ͺ DRY RUN: counting affected records only. Re-run with --apply to make changes."
61+
fi
62+
echo "πŸ“… Plausible year range: [${MIN_YEAR}, ${MAX_YEAR}]"
63+
echo ""
64+
65+
run_neo4j() {
66+
docker exec "${NEO4J_CONTAINER}" cypher-shell -u "${NEO4J_USER}" -p "${NEO4J_PASSWORD}" "$@"
67+
}
68+
69+
run_pg() {
70+
docker exec "${POSTGRES_CONTAINER}" psql -U "${POSTGRES_USER}" -d "${POSTGRES_DB}" -t -A "$@"
71+
}
72+
73+
# ── Neo4j ──────────────────────────────────────────────────────────────────
74+
echo "πŸ” Checking Neo4j container '${NEO4J_CONTAINER}' is running..."
75+
if ! docker inspect --format '{{.State.Running}}' "${NEO4J_CONTAINER}" 2>/dev/null | grep -q true; then
76+
echo "❌ Container '${NEO4J_CONTAINER}' is not running. Start it first with: docker compose up -d neo4j"
77+
exit 1
78+
fi
79+
80+
for label in Release Master; do
81+
COUNT=$(run_neo4j --format plain \
82+
"MATCH (n:${label}) WHERE n.year IS NOT NULL AND (n.year < ${MIN_YEAR} OR n.year > ${MAX_YEAR}) RETURN count(n) AS n;" |
83+
tail -1)
84+
echo " ${label}: ${COUNT} node(s) with an implausible year."
85+
if [[ "${APPLY}" -eq 1 && "${COUNT}" -gt 0 ]]; then
86+
echo " πŸ”„ Nulling ${label}.year for out-of-range values..."
87+
run_neo4j \
88+
"MATCH (n:${label})
89+
WHERE n.year IS NOT NULL AND (n.year < ${MIN_YEAR} OR n.year > ${MAX_YEAR})
90+
CALL { WITH n SET n.year = null } IN TRANSACTIONS OF 50000 ROWS;"
91+
fi
92+
done
93+
echo ""
94+
95+
# ── PostgreSQL ───────────────────────────────────────────────────────────────
96+
echo "πŸ” Checking PostgreSQL container '${POSTGRES_CONTAINER}' is running..."
97+
if ! docker inspect --format '{{.State.Running}}' "${POSTGRES_CONTAINER}" 2>/dev/null | grep -q true; then
98+
echo "❌ Container '${POSTGRES_CONTAINER}' is not running. Start it first with: docker compose up -d postgres"
99+
exit 1
100+
fi
101+
102+
for table in releases masters; do
103+
COUNT=$(run_pg -c \
104+
"SELECT count(*) FROM ${table}
105+
WHERE data->>'year' ~ '^[0-9]+\$'
106+
AND ((data->>'year')::int < ${MIN_YEAR} OR (data->>'year')::int > ${MAX_YEAR});")
107+
echo " ${table}: ${COUNT} row(s) with an implausible year."
108+
if [[ "${APPLY}" -eq 1 && "${COUNT}" -gt 0 ]]; then
109+
echo " πŸ”„ Setting ${table}.data->'year' to null for out-of-range values..."
110+
run_pg -c \
111+
"UPDATE ${table}
112+
SET data = jsonb_set(data, '{year}', 'null'::jsonb)
113+
WHERE data->>'year' ~ '^[0-9]+\$'
114+
AND ((data->>'year')::int < ${MIN_YEAR} OR (data->>'year')::int > ${MAX_YEAR});"
115+
fi
116+
done
117+
echo ""
118+
119+
if [[ "${APPLY}" -eq 1 ]]; then
120+
echo "βœ… Cleanup complete."
121+
else
122+
echo "βœ… Dry run complete. Re-run with --apply to null the years counted above."
123+
fi

0 commit comments

Comments
Β (0)