Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ __pycache__/
# Backup / scratch files
*.bak

# Refactor scratchpad — running notes, intentionally not tracked
docs/contract/REFACTOR_NOTES.md

# Distribution / packaging
.Python
build/
Expand Down
5 changes: 4 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ def _get_release():

# List of patterns, relative to source directory, that match files and
# directories to ignore when looking for source files.
exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store']
# 'contract' holds internal refactor-planning docs (target API contract +
# dependency map), generated by docs/contract/build_*.py — not part of the
# published site, so keep them out of the Sphinx toctree.
exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store', 'contract']

# The name of the Pygments (syntax highlighting) style to use.
pygments_style = 'sphinx'
Expand Down
38 changes: 38 additions & 0 deletions docs/contract/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# TCRI refactor contract

Internal planning/design docs for the `tcri` API refactor (target: a standalone scverse-ecosystem
package, "Door A"). **Not** part of the published Sphinx site (excluded via `exclude_patterns` in
`docs/conf.py`). Structural reference is the `../grafiti` package.

## ⚠ Read first — governance

| File | What it is |
|---|---|
| **`REFACTOR_HISTORY.md`** | Single source of truth: the **Hard Rules**, the chronology, and all settled decisions (§2). Read before touching anything. |
| **`REDO_LIST.md`** | The corrected **disposition map** (keep / drop) + the clean re-implementation plan. |

**Hard Rules (never violate):** the `example/` notebooks are **disposable** — never read them or use them
for any decision. **Non-core = DROP (delete); nothing moves to `examples/`.** Disposition is decided by one
test: *is it core?*

## The authoritative pair

| File | What it is |
|---|---|
| **`tcri_api_and_responsibilities.md`** | Final API surface + per-function math/stats spec + prior-vs-mean resolution. **Clean** (post-recovery). |
| **`tcri_implementation_plan.md`** | Ordered PR sequence + model→AnnData streamline + GPU architecture + testing/CI. **Clean** (post-recovery). |
| `tcri_consistency_sufficiency.md` | Argument consistency deltas + sufficiency-from-responsibility (clean re-derivation). |

## Supporting artifacts

| File | What it is |
|---|---|
| `tcri_function_inventory.md` (+ data, `build_tcri_inventory.py`) | 131-function labeled inventory |
| `tcri_api_contract.*`, `tcri_dependency_map.*` (+ build scripts) | early target API + dependency graph |

## `_quarantine/` — do not use

Earlier versions of the two authoritative docs + their audit data, contaminated by a **notebook-caller-census
leak** (a workflow prompt treated the disposable notebooks as an authority on what to keep, resurrecting
`gene_entropy`/`probability_ternary`). Their **disposition sections are wrong**; their math/design was clean and
was salvaged into the current authoritative docs. Kept only as a record. See `REFACTOR_HISTORY.md`.
61 changes: 61 additions & 0 deletions docs/contract/REDO_LIST.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Redo list — clean re-implementation after the notebook-census leak

Ordered work to redo, from the leak point onward. Each item is re-implemented **clean** (Hard Rules in
`REFACTOR_HISTORY.md`: no notebooks, no `examples/`, disposition = "is it core"). Check each against
`REFACTOR_HISTORY.md` §2 as it lands.

## What we did since the leak point (to re-implement)
1. **Corrected disposition map** (below) — the foundation the two docs got wrong. **Done in this file.**
2. **API + Function-Responsibility doc** — rebuild clean from the quarantined version: keep its (clean)
math/stats spec, prior-vs-mean resolution, per-arg math, `covariate=None`=all semantics; **replace §11 and every
disposition/`examples`/notebook reference** with the map below.
3. **Implementation Plan doc** — rebuild clean: keep the (clean) PR sequence, model→anndata streamline, GPU
architecture, testing/CI; **delete Phase 0's "caller census," every "move to examples," and every "N notebook
uses" justification**; notebook rewrite is a downstream chore, never a disposition driver.
4. **Fold the (clean) consistency findings** into the API doc: `random_state` naming; canonical arg order;
`distance_metric` (not `metric`); `palette` (not `phenotype_colors`); `normalized` (not `normalize`);
`order=`/`hue_order=` parity; `figsize` unify; keyword-only after `*`; `show=` on public `pl`; American spelling.
5. **Re-derive sufficiency from RESPONSIBILITY only** — a kept function gets an arg iff its own job needs it
(`random_state` on sampling fns; `clones=` on per-clone metrics; `order=` for plot category ordering).
**Drop every notebook-parity item** (`gene_entropy`, `probability_ternary`, `weighted=`, `minimum_clone_size=`,
`base=`, `covariate_key=`, `pair_on=`-for-notebooks, etc.) unless responsibility independently requires it.
6. **Necessary/sufficient audit** of the rebuilt docs (consistency matrix over the surface; sufficiency vs responsibility).
7. **Final audit** of everything against `REFACTOR_HISTORY.md`.

---

## CORRECTED DISPOSITION MAP (authoritative; supersedes quarantined §11 / §3)

Decided by "is it core," never by callers. `examples/` does not appear. Allowed non-drop destinations: a kept
namespace, `diag/`, `docs/` (PGM only), or a private helper module.

### KEEP — core surface
- **ml:** `TCRIModel` = `setup_anndata`, `train`, `get_latent_representation`, `predict` (was `get_cell_phenotype_probs`),
`get_p_ct`; private internals `TCRIModule` / priors / classifier / training-plan / `build_archetypes`.
- **pp:** `register_model`→`model.to_anndata` (streamlined), `group_singletons`, `clone_size`.
- **tl:** `joint_distribution` (+ top-level `tcri.joint_distribution`), `clonotypic_entropy`, `phenotypic_entropy`,
`mutual_information`, `phenotypic_flux`, `compare_groups`.
- **pl:** `clonotypic_entropy`, `phenotypic_entropy`, `mutual_information`, `phenotypic_flux`; private
`_sankey` (`SankeyNode` + `_phenotype_mass_per_clone`), `_metric_boxplot` (was `tcri_boxplot`), `_colors`/`resolve_palette`.
- **diag:** `joint_distribution_ppc` (fixed `compare_joint_distribution`), `phenotype_calibration`,
`reconstruction_ppc`, `permutation_null`, `loss` (was `plot_loss`), `archetypes` (was `plot_archetypes`).
- **ut:** `save_tcri_session`, `load_tcri_session`.
- **shared:** `_keys`, `_console`, `_stats`, `_distance`.

### TRANSFORM (kept, relocated — NOT examples)
- `compare_joint_distribution` → `diag.joint_distribution_ppc` (fixed). `plot_loss`→`diag.loss`;
`plot_archetypes`→`diag.archetypes`. `build_nested_tcri_pgm`/`draw_tcri_pgm_nested` → **`docs/`** figure script.
- `register_phenotype_key`/`register_clonotype_key`/`_compute_logits_and_prior` → folded into `to_anndata` (private).
- `dkl` → `_distance.kl_divergence`. `tcri_boxplot` → private `_metric_boxplot`.

### DROP — deleted (NOT moved to examples)
`clonality`, `probability_distribution`, `bayesian_mutual_information`, `polar_plot`, `compare_phenotypes`,
**`probability_ternary`** (was wrongly kept), **`gene_entropy`** (was wrongly →examples), **`top_clone_umap`**,
**`clone_size_umap`**, **`plot_phenotype_probabilities`** (all three were wrongly →examples), `mi_compare`,
`delta_clonotypic_entropy`, `delta_entropy_table`, `flux_table`, `clonotypic_entropy_base`, `ridge_delta_entropy`,
`classify_phenotypes`, `get_latent_embedding`, `group_small_clones`, `register_probability_columns`,
`remove_meaningless_genes`, `clone_fraction`, `probabilities`, `_ent`, `_ascii_hist` (+ all `graph=`/ASCII paths),
`SankeyNode.hex_to_rgb`, `write_adata_safely`, `_pop_nonserializables`, the plural `*_entropies` shims, the `*_tl`/
`centropy`/`pentropy` aliases.

> The 6 items in **bold** are the ones the leak wrongly kept-or-moved-to-examples. They are DROPPED.
130 changes: 130 additions & 0 deletions docs/contract/REFACTOR_AGENDA.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
# TCRI Refactor — Agenda, Tasklist & Diary (LIVING DOC)

**Update this every working session. Refer back to it before starting anything.** It is the operational
tracker + running diary for the whole refactor. The detailed spec lives in `tcri_api_and_responsibilities.md`
+ `tcri_implementation_plan.md`; the rules/history in `REFACTOR_HISTORY.md`; the scratch/deferred pile in
`REFACTOR_NOTES.md` (not checked in).

## How to use this doc
1. **Removal is a hard bar.** The default failure mode is keeping old code around and over-engineering to
preserve it. **Do the opposite.** Every function in the Removal Ledger must actually be deleted, its
`__all__`/import sites cleaned, and the checkbox ticked. If keeping something "just in case" feels tempting —
don't. Delete it; git has it.
2. **Never read the `example/` notebooks.** They are disposable and are an *output* of the refactor, never an
input. No caller census, no "is-it-used," no "sufficiency."
3. **After every PR, run the Standing Audit** (below) and write a diary entry.
4. **Frequent audits**: at minimum after each PR; ideally mid-PR when a component is touched. Log them in the
Audit Log.
5. **Usability is a first-class check** — every session ask "is this easier to use than before?"

## Standing Audit (run after each PR — copy into the diary entry)
- [ ] **Removed everything slated?** (cross-check the Removal Ledger; `__all__` + import-sites clean; `import tcri` green)
- [ ] **Added everything wanted?** (the PR's deliverables all present)
- [ ] **Tests:** what components can now be unit-tested that weren't? Added?
- [ ] **Streamline:** any duplication / dead branch / needless complexity spotted? Removed or logged?
- [ ] **Usability:** simpler signatures / clearer errors / fewer steps than before?
- [ ] **Contract conformance green?** (`test_contract_conformance`)
- [ ] Diary entry written; ledger + statuses updated.

## Status legend: ☐ todo · ◐ in progress · ✅ done · ⚠ blocked

## PR Agenda
| # | PR | Status | Risk | Depends | Gate |
|---|---|---|---|---|---|
| 0 | Contract freeze + CI scaffolding | ✅ | none | — | conformance green |
| 1 | Shared helpers + `_keys` | ✅ | low | 0 | existing tests green |
| 2 | Safe deletions | ☐ | very low | 1 | import-graph clean |
| 3 | Model module split | ☐ | low | 1 | model/pyro tests green |
| 4 | Model→AnnData streamline | ☐ | HIGH | 1,3 | session round-trip |
| 5 | Engine consolidation | ☐ | HIGH | 4 | joint identities |
| 6 | Metric-API consolidation | ☐ | HIGH | 5 | metric tests |
| 7 | Plotting split + pl twins | ☐ | medium | 6,1 | twins render |
| 8 | `diag/` seeding | ☐ | low-med | 4,5 | PPC columns |
| 9 | PGM→docs; utils finalize | ☐ | low | 1,8 | import green sans daft |
| 10 | Notebook rewrite (fresh) | ☐ | low | 4–8 | nbmake tutorial |
| 11 | Public API + scverse CI | ☐ | low-med | all | ecosystem checklist |

## Removal Ledger (the hard bar — every one MUST end deleted)
Tick only when the symbol is gone from source AND `__all__`/imports AND `import tcri` is green.

**Phase 2 (dead / out-of-scope):**
- [ ] `pp.get_latent_embedding` · [ ] `pp.group_small_clones` · [ ] `pp.register_probability_columns`
- [ ] `pp.remove_meaningless_genes` · [ ] `pp.gene_entropy` · [ ] `pp.classify_phenotypes`
- [ ] `pl.polar_plot` · [ ] `pl.probability_distribution` · [ ] `pl.bayesian_mutual_information`
- [ ] `metrics._ent` · [ ] `tl.clone_fraction` · [ ] `metrics.dkl` (→ `_distance.kl_divergence`)
- [ ] `ut.probabilities` (+ its `_plotting.py:18` import, same PR) · [ ] `SankeyNode.hex_to_rgb`

**Phase 4 (folded into `to_anndata` / session):**
- [ ] `pp.register_model` (→ `model.to_anndata`) · [ ] `pp.register_phenotype_key` · [ ] `pp.register_clonotype_key`
- [ ] `pp._compute_logits_and_prior` · [ ] `ut.write_adata_safely` · [ ] `ut._pop_nonserializables`
- [ ] uns keys `tcri_manager`, `tcri_clone_key`, `tcri_phenotype_key`, obsm `X_tcri_phenotypes`

**Phase 5/6 (consolidated away — delete WITH replacement, never before):**
- [ ] `pp.joint_distribution_posterior` (→ unified `joint_distribution`) · [ ] `metrics._mi_from_joint` (→ `_mutual_information`)
- [ ] `tl.mi_compare` (→ `compare_groups`) · [ ] `tl.delta_clonotypic_entropy` · [ ] `tl.delta_entropy_table` · [ ] `tl.flux_table`
- [ ] `tl.clonotypic_entropy_base` · [ ] `tl.clonality` · [ ] `tl.dkl` local `dkl_func`
- [ ] plural `*_entropies` shims · [ ] `metrics/` package (after migration to `tools/`)

**Phase 7 (non-core plots — DROP, not to examples):**
- [ ] `pl.probability_ternary` · [ ] `pl.top_clone_umap` · [ ] `pl.clone_size_umap` · [ ] `pl.plot_phenotype_probabilities`
- [ ] `pl.compare_phenotypes` · [ ] `pl.ridge_delta_entropy` · [ ] `pl.flux` boxplot · [ ] `pl.clonality` plot
- [ ] `pl.tcri_boxplot` (→ private `_metric_boxplot`) · [ ] `pl.set_color_palette` (→ `resolve_palette`)
- [ ] `pl.plot_pheno_sankey` (→ private `_sankey`) · [ ] leaked aliases `centropy`/`pentropy`/`*_tl`

**Phase 9 (out of the package):**
- [ ] `ut.build_nested_tcri_pgm` (→ `docs/`) · [ ] `ut.draw_tcri_pgm_nested` (→ `docs/`) · [ ] `daft` runtime dep

**Phase 3/9 (model/utils cleanup):**
- [ ] `_ascii_hist` (+ all `graph=`/ASCII paths) · [ ] `ml.plot_loss` (→ `diag.loss`) · [ ] `ml.plot_archetypes` (→ `diag.archetypes`)

---

# DIARY

Template per PR: **Goal · Status · What happened · Issues & fixes · Added ✓ · Removed ✓ (hard bar) · Test opportunities · Streamline opportunities · Usability.**

## PR 0 — Contract freeze + CI scaffolding · ✅ done (branch `refactor/pr0-contract-freeze`)
- **Goal:** freeze the target public API as a `.pyi` contract; add signature-drift + import-smoke CI. Zero package behavior change.
- **What happened:** hand-authored `tcri/_contract.pyi` (27 target functions, namespaced `tl/pp/pl/diag/ut` + `TCRIModel`; locked decisions baked in — `n_samples=250`, `weighted=False`, `use_logits`, `normalize_mode`). Ported grafiti's AST signature logic into `tests/test_contract_conformance.py` (contract-parses · live-vs-contract for `IMPLEMENTED` · unimplemented worklist · import-smoke). `IMPLEMENTED={}` (nothing migrated yet). **Full suite: 26 passed, 1 skipped** (23 existing + PR0; additive, zero regressions).
- **Issues & fixes:** `tl`/`pl` share function names → used **namespace container classes** in the `.pyi` so both twins declare cleanly; parser keys as `Namespace.func`; `_strip_receiver` drops `self`/`cls` for the future `TCRIModel` method checks.
- **Added:** ✅ `tcri/_contract.pyi` ✅ `tests/test_contract_conformance.py` ✅ import-smoke
- **Removed (hard bar):** n/a (additive). The contract lists ONLY the kept surface → the 27 declared + the Removal Ledger are the two halves of "done."
- **Test opportunities:** conformance is now a live guardrail; each future PR onboards its functions into `IMPLEMENTED` and drift fails CI. TODO: add `--nbmake` (Phase 10) and multi-py import-smoke in CI yaml.
- **Streamline:** none this PR.
- **Usability:** `_contract.pyi` doubles as the one-screen human-readable target signature reference.
- **Standing Audit:** removed✅(n/a) · added✅ · tests✅(guardrail live) · streamline✅(none) · usability✅ · conformance✅ green · diary✅.
- **Committed** on branch `refactor/pr0-contract-freeze`.

## PR 1 — Shared helpers + `_keys` · ✅ done
- **Goal:** create `_keys`/`_console`/`_stats`/`_distance`; then adopt `_keys` at every read/write site, dedup console, move stats out of utils. API unchanged.
- **What happened (foundation, done):** created the 4 helper modules — `_keys.py` (all uns/obsm/obs constants incl. NEW `GATE_PROB`/`CLASSIFIER_TEMPERATURE`; legacy keys listed for the removal step); `_console.py` (single `_ok/_info/_warn/_fin` + all ANSI aliases both spellings); `_stats.py` (`stars`/`auc_and_label_permutation`/`bootstrap_auc` + **NEW** true `hdi`, `eti`, `prob_direction`, `mann_whitney`); `_distance.py` (`kl_divergence`/`l1_distance`/`jensen_shannon` + `phenotype_distance` dispatch, single eps, bits). Added `tests/test_helpers.py` (8 tests). Suite green.
- **Issues & fixes:** **`hdi` off-by-one** — first version spanned `ceil(prob·n)+1` points, so on a right-skewed sample it returned the full range (HDI==ETI). Caught by a sanity assertion; fixed to the arviz `floor(prob·n)` window → now correctly hugs the low-mass region (`HDI=(0,0.2)` vs `ETI=(0,2.12)`). A concrete "validate the math up front" catch.
- **Added:** ✅ `_keys.py` ✅ `_console.py` ✅ `_stats.py` ✅ `_distance.py` ✅ `tests/test_helpers.py`
- **Removed / deduped (hard bar):** ✅ console dedup — deleted the **12** copied `_ok/_info/_warn/_fin` defs (metrics+preprocessing+plotting) → one `_console`. ✅ moved `stars`/`auc_and_label_permutation`/`bootstrap_auc` out of `utils` → `_stats` (plotting repointed; dead `stars` import dropped). Suite green (34 passed).
- **Test opportunities:** ✅ done — 8 unit tests for pure helpers that were previously embedded/untestable.
- **Streamline:** foundation enables deleting ~3 console dupes + 2 `dkl` copies + the `utils` stats block on adoption.
- **Usability:** internal only this step.
- **`K.*` migration (done):** replaced **85** canonical key literals with `K.*` across preprocessing/metrics/plotting/utils via a verified script (model=0; only legacy keys there); added `test_no_canonical_key_literals` guard — none remain. `dkl` reassigned (dead `metrics.dkl`→Phase 2; `flux` inner→Phase 6). Legacy keys left as literals until their removal phases. **PR1 COMPLETE — 35 passed / 1 skipped.**

## PR 2 — Safe deletions · ☐ todo
_(diary to be filled — this is a REMOVAL PR; the ledger Phase-2 block must be fully ticked)_

## PR 3 — Model module split · ☐ todo
## PR 4 — Model→AnnData streamline · ☐ todo
## PR 5 — Engine consolidation · ☐ todo
## PR 6 — Metric-API consolidation · ☐ todo
## PR 7 — Plotting split + pl twins · ☐ todo
## PR 8 — diag/ seeding · ☐ todo
## PR 9 — PGM→docs; utils finalize · ☐ todo
## PR 10 — Notebook rewrite · ☐ todo
## PR 11 — Public API + scverse CI · ☐ todo
- **Logged test (from grafiti parity):** once `pl.__all__` exists, add a conformance assertion `set(pl.__all__) == {pl entries in _contract.pyi}` — catches *extra/missing* plot functions (whole-surface), not just signature drift on onboarded ones. (tcri's namespaced `.pyi` checks drift incrementally via `IMPLEMENTED`; this closes the whole-surface gap grafiti gets from its markdown+`__all__` channel.)

---

# AUDIT LOG
_(dated entries; what was audited, findings, actions)_

- **(PR0 ✅):** agenda + removal ledger established; standing-audit checklist defined. Contract frozen (27 fns) + conformance guardrail live. Full suite 26 passed / 1 skipped, zero regressions.
- **(PR1 ◐):** shared-helper foundation created (`_keys`/`_console`/`_stats`/`_distance`) + 8 unit tests. Caught & fixed an `hdi` off-by-one before it shipped. **Adoption pending** (dedup, stats-move, `K.*` migration) — no ledger items ticked yet; foundation is additive, suite green. Logged: key-literal test (PR1), `pl.__all__` whole-surface test (PR11).
- **(PR0+PR1 multi-agent audit — 3 lenses):** verdict FIX. Caught a real regression — the `K.*` find/replace over-reached into **10** display/warning/docstring strings (`register_model`/`load_tcri_session` printed `"K.X_LOGITS"` etc.). **Fixed:** restored readable key text in all 10 (AST-span, delimiter-safe); made the key-literal guard **AST-based** (checks real subscripts/`.get`, ignores prose); removed 3 dead `utils` imports the audit flagged. Suite 35 passed. Two non-blocking items deferred to `REFACTOR_NOTES` (contract↔api-doc reconciliation; helper-name canonicalization) — noted in the PR body.
Loading
Loading