Skip to content

Commit 24a1fa8

Browse files
committed
docs(reviews): mark 05/06/07 findings as ADDRESSED (subset)
Cross-reference every commit in this sprint to the review item it addresses. Also flag what's deferred and why.
1 parent b0b4a4d commit 24a1fa8

1 file changed

Lines changed: 44 additions & 0 deletions

File tree

docs/reviews/ADDRESSED.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,51 @@ This document cross-references every review finding addressed in the
4545

4646
* **Auth / CORS / rate-limit on the FastAPI shim** — local-only use,
4747
explicitly excluded.
48+
* **Dockerfile / Helm / K8s manifests** (Review 07 §5) — local-only use.
49+
* **OpenTelemetry tracing** (Review 07 §4) — overkill for current scope.
4850

4951
## Skipped with rationale
5052

5153
* **Review 02 §4.1** (ServerClient drop-in) — see table above.
54+
55+
## Review 05 — FAANG architectural review
56+
57+
| § | Title | Commit |
58+
|---|-------|--------|
59+
| 4.1 | Exception hierarchy (`TinkerNeMoGymError` + subclasses) | `feat(errors): exception hierarchy` |
60+
| 4.1 | Replace substring-match dispatch with `isinstance(SamplerNotReadyError)` | `refactor(server): use SamplerNotReadyError instead of substring-match` |
61+
| 4.1 | Wrap training-call errors as `TrainingError` with context | `refactor(trainer): wrap training-call errors as TrainingError with context` |
62+
| 4.2 | `py.typed` PEP 561 marker | `chore(packaging): add py.typed marker` |
63+
| 4.3 | Populate top-level `__init__.py` (public API + aliases) | `feat(api): re-export public symbols + stable aliases in __init__.py` |
64+
| 4 (D1) | `service_client_factory` DI seam | `feat(trainer): preflight reachability checks ...` (introduced) + `feat(trainer): service_client_factory DI seam for test/extension` (finalised) |
65+
| 4 (D2) | `schema_version` field with upgrade-path validation | `feat(config): schema_version field with upgrade-path validation` |
66+
| 4 (D3) | `SamplingClientProtocol` + `TrainingClientProtocol` | `feat(interfaces): SamplingClientProtocol + TrainingClientProtocol` |
67+
68+
## Review 06 — Library-author review
69+
70+
| § | Title | Commit |
71+
|---|-------|--------|
72+
| 6 (B1) | `tinker_nemogym.train()` sync wrapper | `feat(api): tinker_nemogym.train() sync wrapper` |
73+
| 6 (B2) | Eager config validation (paths + model id regex) | `feat(config): eager validation of paths + base_model pattern` |
74+
| 6 (B3) | `CHANGELOG.md` | `docs: CHANGELOG.md + quickstart.md` |
75+
| 6 (B4) | `docs/quickstart.md` (programmatic-first) | `docs: CHANGELOG.md + quickstart.md` |
76+
77+
## Review 07 — Production operability review
78+
79+
| § | Title | Commit |
80+
|---|-------|--------|
81+
| 3 (C1) | Structured logger + `run_id` contextvar propagation | `feat(utils): structured logging + run_id contextvar propagation` |
82+
| 3 (C2) | Preflight reachability checks with actionable errors | `feat(trainer): preflight reachability checks with actionable errors` |
83+
| 3 (C3) | Resume-state metadata artifact (`<label>.meta.json`) | `feat(checkpoint): resume-state metadata artifact` |
84+
85+
## Deferred from this sprint (reviews 05/06/07)
86+
87+
* **Review 05 §3 full roadmap** — we ship the surgical subset (§4.1-4.3,
88+
D1-D3). The broader restructure (splitting the trainer into a driver +
89+
strategy pattern) is high blast-radius; left as a follow-up.
90+
* **Review 06 §3 v0.2 API** — only `train()` + public-surface stabilisation
91+
landed. Streaming callbacks + pluggable loss-fns deferred.
92+
* **Review 07 §4 metrics backend / Prometheus** — we ship structured
93+
logging only; Prometheus / OpenTelemetry are out of scope for a local-only
94+
library. Wandb already covers the production path.
95+
* **Review 07 §5 container / deployment** — local-only per user directive.

0 commit comments

Comments
 (0)