docs: list shap_parallel module + test file in structural docs (#260)#261
Conversation
Follow-up to #259 — the PR itself updated docs/TRAINING_PIPELINE.md, but three structural docs enumerate source modules and unit-test modules verbatim and are now out of date: - CONTRIBUTING.md: add shap_parallel.py to the Project Structure tree and to the Module Responsibilities table (both after rf_metrics.py). - docs/ARCHITECTURE.md: add a shap_parallel.py row to the Module table, matching the neighbouring linked-path + role-sentence + See-cross-ref style, after rf_metrics.py. - docs/TESTING.md: add test_shap_parallel.py to the tests/unit/ layout block, next to the other train-adjacent test modules. .claude/skills/aetherscan-repo-context/SKILL.md also restates the source tree and needs an equivalent one-line insertion, but .claude/ is read-only to Claude Code in CI — that edit is spelled out verbatim in the PR body for a human (CODEOWNER) to apply locally. CLAUDE.md carries only lean always-on rules and does not enumerate modules, so it is intentionally left untouched. Closes #260
|
Claude finished @claude[bot]'s task in 5m 41s —— View job ReviewDocs-only follow-up to #259 that reconciles the three structural inventories the merged PR left drifting. Diff is 4 additions across 3 files, no
FindingsCorrectness / factual accuracy — good.
Nits (both non-blocking):
SKILL.md instructions — the step-by-step for the human CODEOWNER to apply the RecommendationFix nit 1 (the |
|
Heads-up on why this assistant workflow ( Nothing else for this assistant run to do here:
Standing by — I'll respond to the |
|
Ack — this run was re-triggered by the same |
| | [`inference_viz.py`](../src/aetherscan/inference_viz.py) | End-of-run inference visualization suite (ED distributions, galleries, latent projection, summary card). | | ||
| | [`models/vae.py`](../src/aetherscan/models/vae.py), [`models/random_forest.py`](../src/aetherscan/models/random_forest.py) | Model definitions. See [`MODELS.md`](MODELS.md). | | ||
| | [`rf_metrics.py`](../src/aetherscan/rf_metrics.py) | Pure (TF-free) helper `compute_rf_eval_metrics()` — sklearn.metrics-based scalar RF eval metrics that `train.py` persists to `training_stats` (`model_name='rf'`) for the dashboard's RF tab. | | ||
| | [`shap_parallel.py`](../src/aetherscan/shap_parallel.py) | TF-free process-pool wrapper (`parallel_shap`) that chunks the RF SHAP passes across worker processes; each worker rebuilds a stock `TreeExplainer` on its chunk. Called by `train.py::_compute_or_load_shap_values`. See [`TRAINING_PIPELINE.md`](TRAINING_PIPELINE.md). | |
There was a problem hiding this comment.
Trailing padding is ~170 chars past every other row in this table. All other rows in the Module table (lines 46–70) pad their trailing | to roughly column 439–440 (I verified with ^.{450,}$ → only this line matches; ^.{440,}$ → this line + one other). This new row is 605 chars long, so its terminal | sticks out ~165 chars to the right of the rest of the table's right edge — the source-file column alignment convention the rest of the table follows is broken here.
Renders identically in markdown (GitHub doesn't care), but it's an obvious visual anomaly when scrolling the raw source and inconsistent with the neighbouring rf_metrics.py / db/db.py rows.
Two ways to fix, pick one:
- Trim the trailing padding so the terminal
|lands at the same column as its siblings (~col 439). The Role sentence itself (264 chars) is longer than the widest neighbour (main.py/seeding.pyaround 230 chars), so the row will still be a touch wider than average, but it won't sit ~170 chars past the pack. Minimal-touch fix. - Also tighten the Role prose so it fits within the Role column width used by the rest of the table (~350 chars). E.g. drop the redundant "
parallel_shap" (it's the module's only public entry point, theSee TRAINING_PIPELINE.mdlink already scopes it) and collapse the two sentences: "TF-free process-pool wrapper that chunks RF SHAP passes across workers; each worker rebuilds a stockTreeExplainer. Called bytrain.py::_compute_or_load_shap_values. SeeTRAINING_PIPELINE.md." — that's ~215 chars and comfortably fits the existing column.
Either is fine; option 1 is the smallest change if you want to move on.
| │ ├── test_run_state.py # manifest round-trip, stage machine transitions | ||
| │ ├── test_train_utils.py # get_latest_tag ladder, curriculum schedules, archiving | ||
| │ ├── test_train_datasets.py # batched generators: coverage, stratification, alignment | ||
| │ ├── test_shap_parallel.py # positive-class shape normalization + MP-vs-single-process byte-identity (all three passes) |
There was a problem hiding this comment.
Placement is defensible but not the most natural home. The PR body describes the tests/unit/ ordering as "sorted" — it isn't alphabetical, it's pipeline-phase-grouped, and by that grouping test_shap_parallel.py is a closer sibling of test_rf_metrics.py (line 53) than of test_train_datasets.py: both are TF-free RF-adjacent helpers, and their production-code counterparts (rf_metrics.py, shap_parallel.py) are adjacent in both CONTRIBUTING.md and docs/ARCHITECTURE.md in this same PR.
Alternative placement worth considering: move this line to immediately after test_rf_metrics.py on line 53, so the RF/SHAP siblings sit together in TESTING.md the same way their source modules sit together in the other two docs. Purely a consistency call — the current position isn't wrong.
- docs/ARCHITECTURE.md: trim the shap_parallel row + pad its trailing pipe so it matches the Module table's column alignment (was ~165 chars past the right edge). - docs/TESTING.md: move test_shap_parallel.py next to test_rf_metrics.py (both are TF-free RF-adjacent helpers, matching where the source modules sit in CONTRIBUTING.md / ARCHITECTURE.md). Refs #260
|
Internal change — no release note. |
Summary
Follow-up to merged PR #259 (parallelizing RF SHAP explainability). PR #259 updated the one narrative doc that discussed the SHAP figures (
docs/TRAINING_PIPELINE.md), but three structural docs enumerate source modules and unit-test modules verbatim and were out of date. This PR reconciles them:CONTRIBUTING.md— Project Structure tree and Module Responsibilities table both grow ashap_parallel.pyentry afterrf_metrics.py(its natural sibling: another TF-free RF-adjacent helper called bytrain.py).docs/ARCHITECTURE.md— Module table grows ashap_parallel.pyrow afterrf_metrics.py, matching the neighbouring linked-path + role-sentence +See ...cross-ref style.docs/TESTING.md—tests/unit/layout block grows atest_shap_parallel.pyentry next to the other train-adjacent test modules (aftertest_train_datasets.py).Existing formatting conventions are preserved (column alignment in the CONTRIBUTING.md tree/table, ARCHITECTURE.md row style, TESTING.md sorted layout).
Per the issue's contract:
CLAUDE.mdis deliberately left untouched — it carries only lean always-on rules and does not enumeratesrc/aetherscan/*.pymodules or currently mention SHAP anywhere; nothing in it is made factually wrong by feat(train): parallelize RF SHAP explainability across CPU cores #259..pysource file, no SHAP logic, and nodocs/TRAINING_PIPELINE.mdchange in this PR — those are out of scope; the TRAINING_PIPELINE.md subsection was already added by feat(train): parallelize RF SHAP explainability across CPU cores #259..claude/skills/aetherscan-repo-context/SKILL.mdis NOT touched here — that path is read-only to Claude Code in CI. The required edit is spelled out verbatim below for a human (CODEOWNER) to apply locally.SKILL.md edits (apply manually —
.claude/is read-only in CI).claude/skills/aetherscan-repo-context/SKILL.md's Project Structure tree (~lines 106-142) restates the source-module inventory and must also listshap_parallel.py. Because the@claudefollow-up PR cannot write under.claude/, apply this edit verbatim locally.Edit 1 — insert one line into the source tree, immediately after the existing
rf_metrics.pyline:Find:
Replace with:
(No other lines in SKILL.md need to change — it doesn't currently mention SHAP anywhere else, and no other statement in it is contradicted by this PR.)
Step-by-step for the human (CODEOWNER) apply
Because
.claude/is read-only to Claude Code in CI, the SKILL.md edit above cannot be pushed from this PR — it needs to be applied and committed by a human locally on this same branch. @zachtheyek (perCODEOWNERS: * @zachtheyek), please:.claude/skills/aetherscan-repo-context/SKILL.mdand apply Edit 1 exactly as spelled out above (single-line insertion immediately after therf_metrics.pyline in the Project Structure tree).Test plan
pre-commit run --files CONTRIBUTING.md docs/ARCHITECTURE.md docs/TESTING.mdlocally — all applicable hooks (large files / case conflicts / merge conflicts / EOF / trailing whitespace / private key / branch guard / gitleaks) passed; the ruff hooks correctly skipped (no.pyfiles staged).src/aetherscan/shap_parallel.pyandtests/unit/test_shap_parallel.py.rf_metrics.py/test_train_datasets.pyrespectively and preserve the surrounding tree indentation / table column alignment.claude-code-reviewfirst pass lands cleanly (I will wait, then address individual notes on this same PR per the CLAUDE.md review contract).Closes #260