Skip to content

Commit a22119f

Browse files
committed
perf(validate): scope PR validation to the changed records
`app.validate --changed-since BASE` loads only the records changed since BASE plus the whole brand/soc/cpu/gpu sets that others reference, so a PR touching a few files no longer parses ~190k. Slug uniqueness for changed records is checked against every existing file name in the category (no JSON parsing). The reusable validate-data workflow takes an optional `base-sha` and uses it with a shallow checkout; without it the full run is unchanged. Refs GetTechAPI/TechAPI#350
1 parent f90e42c commit a22119f

4 files changed

Lines changed: 128 additions & 20 deletions

File tree

‎.github/workflows/validate-data.yml‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ on:
99
type: string
1010
required: false
1111
default: "main"
12+
base-sha:
13+
description: "PR base SHA; when set only records changed since it are validated"
14+
type: string
15+
required: false
16+
default: ""
1217
workflow_dispatch:
1318
inputs:
1419
data-ref:
@@ -27,10 +32,20 @@ jobs:
2732
repository: GetTechAPI/TechAPI
2833
ref: ${{ inputs.data-ref }}
2934
path: TechAPI
35+
# Scoped runs need the diff back to the merge base, not the full history.
36+
fetch-depth: ${{ inputs.base-sha != '' && 100 || 1 }}
3037
- uses: actions/setup-python@v6
3138
with:
3239
python-version: "3.12"
3340
- name: Validate seed JSON (§9.3, §15.3)
3441
env:
3542
TECHAPI_DATA_DIR: ${{ github.workspace }}/TechAPI/data
36-
run: python -m app.validate
43+
run: |
44+
if [ -n "${{ inputs.base-sha }}" ]; then
45+
git -C TechAPI fetch --no-tags --depth=100 origin "${{ inputs.base-sha }}"
46+
git -C TechAPI merge-base "${{ inputs.base-sha }}" HEAD >/dev/null 2>&1 \
47+
|| git -C TechAPI fetch --no-tags --unshallow origin
48+
python -m app.validate --changed-since "${{ inputs.base-sha }}"
49+
else
50+
python -m app.validate
51+
fi

‎app/validate.py‎

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@
1010

1111
from __future__ import annotations
1212

13+
import argparse
1314
import json
1415
import re
16+
import subprocess
1517
import sys
1618
from pathlib import Path
1719
from typing import Any
@@ -148,6 +150,52 @@ def _load(subdir: str, only: set[str] | None = None) -> list[tuple[str, dict[str
148150
]
149151

150152

153+
# Categories other records reference by slug (brand/soc/cpu/gpu). A scoped run
154+
# still loads these whole (~9k files) so foreign-key checks stay exact.
155+
FK_CATEGORIES = ("brand", "soc", "cpu", "gpu")
156+
157+
158+
def changed_paths(base: str) -> set[str]:
159+
"""Seed paths (``data/`` stripped) changed between ``base`` and HEAD."""
160+
out = subprocess.run(
161+
["git", "diff", "--name-only", f"{base}...HEAD", "--", "data/"],
162+
capture_output=True, text=True, check=True, cwd=DATA_DIR.parent,
163+
).stdout
164+
return {
165+
line[len("data/"):] for line in map(str.strip, out.splitlines())
166+
if line.startswith("data/") and line.endswith(".json")
167+
}
168+
169+
170+
def _check_unique_slugs_scoped(
171+
category: str,
172+
records: list[tuple[str, dict[str, Any]]],
173+
errors: list[str],
174+
) -> None:
175+
"""Scoped uniqueness: changed records vs each other and vs every file *name*.
176+
177+
Filenames equal slugs by convention, so listing names (no JSON parsing) finds
178+
a clash with an unchanged record. A clash where the name differs from the slug
179+
is only caught by the full run (push to develop/main, nightly).
180+
"""
181+
changed = {fname for fname, _ in records}
182+
stems: dict[str, list[str]] = {}
183+
for f in (DATA_DIR / category).rglob("*.json"):
184+
stems.setdefault(f.stem, []).append(str(f.relative_to(DATA_DIR)))
185+
seen: dict[str, str] = {}
186+
for fname, rec in records:
187+
slug = rec.get("slug")
188+
if not isinstance(slug, str):
189+
continue
190+
if slug in seen:
191+
errors.append(f"{fname}: duplicate {category} slug '{slug}' (also in {seen[slug]})")
192+
continue
193+
seen[slug] = fname
194+
for other in stems.get(slug, []):
195+
if other not in changed:
196+
errors.append(f"{fname}: duplicate {category} slug '{slug}' (also in {other})")
197+
198+
151199
def _check_required(
152200
name: str, record: dict[str, Any], required: set[str], errors: list[str]
153201
) -> None:
@@ -234,10 +282,14 @@ def _check_variant_path(
234282
errors.append(f"{fname}: filename must match slug '{rec.get('slug')}'")
235283

236284

237-
def validate() -> list[str]:
285+
def validate(only: set[str] | None = None) -> list[str]:
286+
"""Validate the seed data; with ``only``, just those paths (+ whole FK targets)."""
238287
errors: list[str] = []
239288

240-
loaded = {category: _load(category) for category in CATEGORIES}
289+
loaded = {
290+
category: _load(category, None if category in FK_CATEGORIES else only)
291+
for category in CATEGORIES
292+
}
241293
(brands, socs, phones, tablets, watches, pdas, gpus, cpus,
242294
laptops, monitors, software, websites) = (loaded[category] for category in CATEGORIES)
243295

@@ -247,7 +299,10 @@ def validate() -> list[str]:
247299
gpu_slugs = {rec["slug"] for _, rec in gpus if "slug" in rec}
248300

249301
for category, records in loaded.items():
250-
_check_unique_slugs(category, records, errors)
302+
if only is not None and category not in FK_CATEGORIES:
303+
_check_unique_slugs_scoped(category, records, errors)
304+
else:
305+
_check_unique_slugs(category, records, errors)
251306

252307
for fname, rec in brands:
253308
_check_required(fname, rec, BRAND_REQUIRED, errors)
@@ -413,13 +468,13 @@ def validate() -> list[str]:
413468
return errors
414469

415470

416-
def run() -> int:
471+
def run(only: set[str] | None = None) -> int:
417472
# The ✅/❌ status glyphs must not crash on legacy consoles (e.g. cp949).
418473
try:
419474
sys.stdout.reconfigure(encoding="utf-8") # type: ignore[union-attr]
420475
except Exception:
421476
pass
422-
errors = validate()
477+
errors = validate(only)
423478
if errors:
424479
print(f"❌ Data validation failed ({len(errors)} issue(s)):")
425480
for err in errors:
@@ -430,4 +485,10 @@ def run() -> int:
430485

431486

432487
if __name__ == "__main__":
433-
sys.exit(run())
488+
parser = argparse.ArgumentParser(description="Validate TechAPI seed data")
489+
parser.add_argument(
490+
"--changed-since", metavar="BASE",
491+
help="validate only records changed vs BASE (full run when omitted)",
492+
)
493+
args = parser.parse_args()
494+
sys.exit(run(changed_paths(args.changed_since) if args.changed_since else None))

‎app/verify/cli.py‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,13 @@
1313

1414
import argparse
1515
import json
16-
import subprocess
1716
from collections import Counter, defaultdict
1817
from collections.abc import Iterator
1918
from datetime import UTC, datetime
2019
from pathlib import Path
2120
from typing import Any
2221

23-
from app.validate import DATA_DIR
22+
from app.validate import changed_paths
2423

2524
from . import crossref, http_check, ledger, offline, promote, wikidata
2625
from .common import (
@@ -44,17 +43,7 @@ def _now_iso() -> str:
4443

4544
def _changed_data_slugs(base: str = "origin/main") -> set[str]:
4645
"""Changed seed paths against the PR merge base; git errors must stay visible."""
47-
out = subprocess.run(
48-
["git", "diff", "--name-only", f"{base}...HEAD", "--", "data/"],
49-
capture_output=True, text=True, check=True, cwd=DATA_DIR.parent,
50-
).stdout
51-
# strip leading "data/" so it matches Record.path
52-
paths = set()
53-
for line in out.splitlines():
54-
line = line.strip()
55-
if line.startswith("data/") and line.endswith(".json"):
56-
paths.add(line[len("data/"):])
57-
return paths
46+
return changed_paths(base)
5847

5948

6049
def _iter_selected(

‎tests/unit/test_validate_scoped.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
"""Scoped validation: FK targets stay whole, duplicates are still caught."""
2+
3+
import json
4+
5+
from app import validate
6+
7+
BRAND = {"slug": "acme", "name": "Acme", "country": "US",
8+
"categories": ["smartphone-oem"], "source_urls": ["https://example.com"]}
9+
10+
11+
def _put(root, rel, rec):
12+
f = root / rel
13+
f.parent.mkdir(parents=True, exist_ok=True)
14+
f.write_text(json.dumps(rec), encoding="utf-8")
15+
16+
17+
def _site(tmp_path, monkeypatch):
18+
monkeypatch.setattr(validate, "DATA_DIR", tmp_path)
19+
_put(tmp_path, "brand/us/acme.json", BRAND)
20+
_put(tmp_path, "website/a/one.json", {"slug": "one"})
21+
_put(tmp_path, "website/a/two.json", {"slug": "two"})
22+
23+
24+
def test_scoped_only_checks_changed_files(tmp_path, monkeypatch):
25+
_site(tmp_path, monkeypatch)
26+
full = validate.validate()
27+
scoped = validate.validate({"website/a/one.json"})
28+
# full run flags both incomplete websites, scoped run only the changed one
29+
assert any("two.json" in e for e in full)
30+
assert not any("two.json" in e for e in scoped)
31+
assert any("one.json" in e for e in scoped)
32+
33+
34+
def test_scoped_finds_duplicate_slug_in_unchanged_file(tmp_path, monkeypatch):
35+
_site(tmp_path, monkeypatch)
36+
_put(tmp_path, "website/b/one.json", {"slug": "one"})
37+
errs = validate.validate({"website/b/one.json"})
38+
assert any("duplicate website slug 'one'" in e for e in errs)
39+
40+
41+
def test_scoped_empty_change_set_passes_fk_categories_only(tmp_path, monkeypatch):
42+
_site(tmp_path, monkeypatch)
43+
assert not any("website" in e for e in validate.validate(set()))

0 commit comments

Comments
 (0)