refactor(weekly-scan): filter the model list before sharding it

[assaftibm]

Problem

generate_weekly_shards.py chunks a downloads-ordered list into fixed-size shards (250 x1 / 100 x2 / 50 x4), and weekly_test.py then filtered within each shard. The filtered-out models cluster — config-class families cluster by download count — so surviving counts per shard varied wildly. Some scan jobs finished in minutes while others ran for hours against the same nominal shard size.

Fix

Filter before _chunk(), so shard size is a count of real evaluations.

Shard sizes, tier routing, the matrix shape and max-parallel are all unchanged — the only difference is what _chunk() receives. Expect fewer, fuller jobs, which also moves further from the 256-job matrix cap.

Filtering upstream means the filter has to be shared, so:

• New model_prefilter.py — pure function, returns a three-way partition. The skip-window case is deliberately separate from the terminal categories: those models already have a recent row, so writing another would duplicate it or be silently swallowed by the sink's guard.

• --model-list-file is now required in weekly_test.py; --top-k and the fetch branches are gone. All three existing call sites already pass exactly --mode and --model-list-file, so no caller breaks. --mode stays — it selects the batch size, the model class, the pipeline, and the table.

• New --fetch flag on weekly_test.py for manual runs with no shard file: fetch by --mode, apply the same prefilter_models, record the terminal rows, evaluate the survivors — one invocation. Mutually exclusive with --model-list-file, one required. --top-k / --max-params are rejected without it rather than silently ignored.

  This is not a return to the unfiltered fetch path this PR removes. That one fetched without filtering, which is what made shards uneven; --fetch runs the shared filter, so both entry points agree on what reaches a worker and the filter still has one implementation.

• generate-matrix gains ClickHouse access — it now reads the skip window and writes the terminal verdict rows itself.

• add_entry's dedup guard becomes opt-out (dedup_guard, keyword-only, default True; import_csv keeps it on). Both weekly sinks pass False: the skip-window rule is evaluated once, upstream, so from then on a write records a decision already taken and a second check could only discard a row the caller chose to write. The skip-window rule is evaluated once, upstream, while the list is built; after that a write records the outcome of a decision already taken, so a second check could only discard a row the caller chose to write — leaving the run with fewer rows than the models it handled and no accounting for the gap. In weekly_test.py the guard would also have consulted a skip set snapshotted when that job started, two hours newer than the producer's, so it wasn't even re-asking the same question. A duplicate is the better failure: ReplacingMergeTree(snapshot_date) collapses same-day duplicates on merge.

Two things found along the way

The in-worker size backstop does not exist. A comment claimed _process_batch kept a too-large check "as a defensive backstop for rows where parameters were unknown at fetch time." It doesn't — the only MAX_NUMBER_PARAMS comparison in the file was the parent's, and FAILURE_CATEGORY_MODEL_TOO_LARGE appears in _process_batch solely in a traceback-suppression list. So an oversized model the fetcher couldn't size surfaces as cpu_load_failed or a worker timeout, exactly as before this change. Comments corrected rather than a backstop invented.

result_sink.py was string-duplicating "hardware_exception" with a comment explaining it dodged a circular import with weekly_test.py. The new leaf constants module removes the need, so the workaround is gone.

Behaviour changes worth knowing

• Cancel safety (accepted). Terminal rows are written at fetch time, so their 10-day skip window starts then. Cancel a run after generate-matrix succeeds — routine, given cancel-in-progress: true — and those models read as "done" for 10 days though no scan ran. Mitigating: these are terminal properties of the checkpoint (a MoE model won't stop being MoE), so re-deriving them weekly is wasted work anyway.
• Processed N/total changes meaning — from "fetched" to "to evaluate", so it now approaches N == total instead of being a small fraction. Confirmed in the integration run below. With the guard off in weekly_test.py, rows written now always equals models handled.
• Empty tiers are now likely (a re-run inside the skip window), where they were near-impossible. Each scan job gained an if: ... != '[]' guard rather than relying on how Actions treats a zero-length matrix.include.
• Skipped rows now land ~2h before scan rows, so a Saturday 22:00 UTC run can straddle midnight and split one logical run across two snapshot_dates. Not corrupting (ReplacingMergeTree), and pre-existing in kind — a 4320-minute scan job can already straddle days.

Verification

Unit tests — the first for any of these modules (tests/test_weekly_prefilter.py, 28 tests). At tests/ root under --noconftest, following test_adapter_coverage.py, because the root conftest imports torch. Covers each branch, precedence (window-skip wins, so no duplicate rows accumulate), input-order preservation, parameter coercion, the exact 14-column field mapping the deleted add_entry blocks produced, and an inspect.signature guard on the positional-construction hazard at clickhouse_db.py:292.

They also verify the import split: 18 pass with no clickhouse_connect installed, 10 sink-backed ones skip. That's what the leaf constants module buys.

Live against the HF API:

• generate_weekly_shards.py --dry-run --top-k 30 --model-type embedding → 30 fetched, 27 kept, 3 dropped not-implemented-adapter; shard JSON valid, no model_info leak; matrix_x4=[] appeared naturally, which is what prompted the if: guards.
• weekly_test.py --fetch --top-k 12 --write-to-csv: wrote a fresh file, recorded 1 not-implemented-adapter row, evaluated 11 of 12, reported Processed 11/11. A second run against the same path was refused with an actionable FileExistsError.
• CsvResultSink is now write-only — it refuses a non-empty existing file, reports nothing as blocking, and defaults dedup_guard to False. So it is an output for no-DB runs, not a second skip-window implementation that behaved differently from ClickHouse for the same input. --fetch --write-to-csv therefore applies no skip window, and says so at runtime.
• Argument contract checked across all 8 valid/invalid flag combinations (neither source, both sources, tuning flags without --fetch).

ruff, black, yamllint clean.

Still needs the pod: one shard through weekly_test.py. The batching path is untouched, but _process_batch runs in a spawn child that re-imports the module, so a broken failure_categories import would surface as a worker crash rather than a parent import error.

Acceptance criterion: on a full run, compare scan-job durations against last week's. Near-uniform durations is the point of the change.

Not in here

Dead get_all_models (result_sink.py:118/:284/:419, zero callers repo-wide) — separate commit. Also still outstanding: the "TEMPORARY debug step" at push-to-clickhouse.yaml:147-185, and test_weekly_DEPRECATED.yaml firing on the same cron slot as the real scan.

🤖 Generated with Claude Code

[BenjSz]

LGTM - I edited one doctring.

[ashokponkumar]

@HarikrishnanBalagopal ptal

[spyre-ci]

❌ spyre-test: failure

:warning: orch *trigger-pr-validation* — *yellow* · arches amd64 · fp amd64=a8f0ea18
L0 deeptools                amd64 ❌
L1 flex                     amd64 ⛔
L2 aiu-toolbox/ibm-aiu-toolbox-e2e amd64 ⛔
L2 spyre-comms              amd64 ⛔
L3 spyre-backend/spyre-backend-dev amd64 ⛔
L4 torch-spyre/torch-spyre-dev amd64 ⛔
L5 hf-adapters/hf-adapters-dev amd64 ⛔