Skip to content

refactor(PR3): split model/_model.py into scvi-style modules + explicit __all__ - #32

Merged
nceglia merged 2 commits into
refactor/pr2-safe-deletionsfrom
refactor/pr3-model-split
Jul 12, 2026
Merged

refactor(PR3): split model/_model.py into scvi-style modules + explicit __all__#32
nceglia merged 2 commits into
refactor/pr2-safe-deletionsfrom
refactor/pr3-model-split

Conversation

@nceglia

@nceglia nceglia commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Stacked on #31 (PR2). Base = refactor/pr2-safe-deletions — GitHub retargets as the stack merges.

Mechanical split — no behavior change. Breaks the 1074-line model/_model.py into scvi-tools-style sibling modules. import tcri green; full suite 36 passed / 1 skipped.

The split

file holds lines
_model.py TCRIModel (BaseModelClass API: setup_anndata, init, train, get_latent_representation, get_cell_phenotype_probs, get_p_ct, …) 462
_module.py TCRIModule — the Pyro model/guide 326
_priors.py MixtureDirichlet, VampPrior 147
_classifier.py PhenotypeClassifier 21
_training.py UnifiedTrainingPlan, build_archetypes 154

Extraction used ast.get_source_segment (formatting-preserving), so each class body is byte-identical to the original modulo the one sanctioned rename. Clean dependency DAG: _classifier/_priors_module_training_model. Every module declares an explicit __all__ (a Phase-3 deliverable): tcri.model.* is pinned to exactly {TCRIModel} — the public API the frozen contract promises. (The old import * incidentally leaked ~17 third-party names — pyro, dist, Encoder, KMeans, … — none tcri-defined and none referenced anywhere; those are no longer surfaced.)

Rename

c2p_mat → clone_phenotype_prior (plan-sanctioned, Phase 3) — 13 word-boundary sites; the unrelated local c2p_torch and the module register_buffer("clone_phen_prior") are untouched.

Incidental cleanup

Rebuilding per-file import headers surfaced 3 provably-dead top-level imports, now dropped: setup_anndata_dsp, cosine_similarity, and the torch.distributions Categorical/Dirichlet/MixtureSameFamily trio (every real use is dist.-prefixed pyro).

New coverage

The construct/train path was entirely uncovered (the trained_model fixture was defined but unused). Added tests/test_model_smoke.py: construct → train (2 epochs) → get_latent_representation / get_p_ct / get_cell_phenotype_probs (asserts shapes + probability normalization), and asserts the rename landed and build_archetypes returns centers and labels. ~1s in-suite.

Deferred (logged)

M5build_archetypes default K=4 vs TCRIModel K=10. Behavior-neutral (the model always passes K=10 explicitly); reconciled with persisted labels when diag.archetypes lands (Phase 8). Not touched here to keep the split purely mechanical.

Audit

A workflow audit ran on this branch — 3 lenses (doc↔code · behavior preservation · plan/contract), each finding adversarially verified (8 agents total). The behavior lens confirmed the split is a pure refactor: class bodies byte-identical to the pre-split monolith (AST diff, modulo the sanctioned rename), zero undefined-names (F821) across all 5 files — proving every per-file import header is complete — and the suite + smoke green.

Verdict: 2 lenses PASS, plan-contract FIX. 5 findings, all confirmed, all LOW/MED — no behavior or correctness defect. Fixed in this branch: the MED — explicit __all__ per module (plan §Phase 3) was omitted — now added to all 5 files, which also resolves the two LOW "surface not byte-for-byte" wording findings (surface pinned to {TCRIModel}).

One LOW item deferred (on the auditor's own recommendation): stale c2p_mat descriptors in the contract generator (build_tcri_contract.py) — bundled with the Phase-8 diag.archetypes/M5 reconciliation, since they describe that future function. Tracked in docs/contract/REFACTOR_NOTES.md.

Stacked so the diff shows only PR3's changes. Living agenda + audit log: docs/contract/REFACTOR_AGENDA.md.

🤖 Generated with Claude Code

nceglia and others added 2 commits July 12, 2026 09:38
Mechanical split (no behavior change) of the 1074-line _model.py into:
  _model.py     TCRIModel (BaseModelClass API)
  _module.py    TCRIModule (pyro model/guide)
  _priors.py    MixtureDirichlet, VampPrior
  _classifier.py PhenotypeClassifier
  _training.py  UnifiedTrainingPlan, build_archetypes

Extraction via ast.get_source_segment (formatting-preserving) along the DAG
_classifier/_priors -> _module -> _training -> _model. _model re-imports all
six moved symbols so the tcri.model.* surface is unchanged. Renamed
c2p_mat -> clone_phenotype_prior (13 sites; c2p_torch + module buffer
clone_phen_prior untouched). Dropped 3 dead top-level imports
(setup_anndata_dsp, cosine_similarity, torch.distributions trio).

Added tests/test_model_smoke.py: construct -> train -> latent/p_ct/predict
(the train path was previously uncovered). Suite 36 passed / 1 skipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR3 audit (workflow, 3 lenses x adversarial verify): behavior + doc-code PASS,
plan-contract FIX. All 5 findings LOW/MED, zero behavior defect (class bodies
byte-identical to the monolith; zero F821; suite green).

Fix the MED: 'explicit __all__ per module' (plan Phase 3, line 279) was omitted.
Add __all__ to all 5 model files; _model pins tcri.model.* to {TCRIModel} (the
frozen-contract public surface), dropping the incidental third-party import-*
leaks and the 3 now-unneeded noqa re-exports. Corrects the diary's overstated
'byte-for-byte surface' wording. Suite 36 passed / 1 skipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant