felt: ngmix-size-columns fresh-eyes review — verified, closed

All three branches independently re-reviewed and re-tested; no code
defects. Report corrected: sp_validation CI is green-but-vacuous on
the cs_util.size import (suite never imports galaxy.py, image carries
released cs-util 0.1.9), so merge order is discipline-enforced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: cailmdaley

.felt/shapepipe/ngmix-size-columns/ngmix-size-columns.md

@@ -1,13 +1,14 @@
 ---
 id: 01KTCQPE4M6S1E4V9WMPCAFCKT
 name: 'ngmix size columns: honest r50 + cs_util converter web'
-status: active
+status: closed
 tags:
     - constitution
     - shapepipe
     - ngmix
 created-at: 2026-06-05T22:30:50.004535516+02:00
-outcome: 'Realized, pending fresh-eyes review: shapepipe fix/ngmix-size-columns (honest r50 + dedupe, pushed), cs_util feat/size-conversions (size web + tests, on cailmdaley fork — no push rights upstream), sp_validation fix/psf-leakage-fwhm (T_to_fwhm bug fixed — moves α(size); pushed). PRs + merge order (cs_util → sp_validation) are Cail''s gesture; see report.html.'
+outcome: |-
+    Realized + fresh-eyes reviewed, no code defects: shapepipe fix/ngmix-size-columns (honest r50 + dedupe, pushed), cs_util feat/size-conversions (size web, 24/24, on cailmdaley fork — no upstream push rights), sp_validation fix/psf-leakage-fwhm (T_to_fwhm bug fixed — moves α(size); pushed). PRs + merge order (cs_util release → sp_validation) are Cail's gesture; caution: sp_validation CI is green-but-vacuous on the cs_util.size import, ordering is discipline-enforced. See report.html.
 shuttle:
     kind: oneshot
     host: candide

.felt/shapepipe/ngmix-size-columns/report.html

@@ -70,10 +70,17 @@ <h1>Honest r50 columns, a shared size web, and the leakage-axis fix.</h1>
 
   <section>
     <div class="section-mark">§ Current state</div>
-    <h2>All three branches landed; PR creation is yours.</h2>
+    <h2>All three branches landed and fresh-eyes reviewed; PR creation is yours.</h2>
 
     <p>Every piece of the constitution's desired state is implemented, tested, and pushed.
-    Three branches across three repos, in dependency order:</p>
+    A second session independently reviewed all three diffs: re-derived both error propagations
+    (galaxy <code>r50·T_err/2T</code> and PSF <code>√(2ln2)·T_err/(4σ)</code> are the same formula —
+    consistent), confirmed zero remaining consumers of the retired <code>_psfo_ngmix</code> size
+    columns (only <code>make_cat</code>, repointed), confirmed the dropped <code>pixel_size</code>
+    kwarg on <code>sigma_to_fwhm</code> has no callers, re-ran the tests (shapepipe size tests pass
+    in the dev container; cs_util 24/24; <code>T_to_fwhm(2.0) → 2.35482</code> reproduced in the
+    sp_validation container against the branch), and verified all three push SHAs. No code defects
+    found. Three branches across three repos, in dependency order:</p>
 
     <table>
       <tr><th>Repo</th><th>Branch</th><th>What it does</th></tr>
@@ -115,8 +122,18 @@ <h2>All three branches landed; PR creation is yours.</h2>
 
     <p><strong>Merge order:</strong> cs_util first (sp_validation's import fails loudly without it —
     intentional fail-fast), then sp_validation; the shapepipe branch is independent and can merge any
-    time. No PRs opened — that's your gesture. cs_util will also need a release (or a git-ref pin) before
-    sp_validation's CI can pass.</p>
+    time. No PRs opened — that's your gesture. cs_util needs a release (or a git-ref pin in
+    sp_validation) before the sp_validation branch is safe to merge.</p>
+
+    <div class="flag">
+      <strong>CI will not police the merge order.</strong> sp_validation's CI is already green on the
+      pushed branch (run 27248208943) — but vacuously: the image installs <em>released</em>
+      <code>cs-util 0.1.9</code> (no <code>size</code> module), and the test suite never imports
+      <code>galaxy.py</code> (tests cover <code>cosmo_val</code>/<code>cosmology</code>/<code>survey</code>/<code>plots</code>
+      only). Merging sp_validation before the cs_util release would pass CI and then raise
+      <code>ImportError</code> at runtime, in <code>extract_info.py</code> / the notebooks. The
+      ordering is enforced by discipline, not automation.
+    </div>
   </section>
 
   <hr class="short">
@@ -150,9 +167,14 @@ <h3>Exact constants over truncated literals</h3>
 
     <h3>Pre-existing test failure, not mine</h3>
     <p><code>test_metacal_is_reproducible_with_fixed_seed</code> fails in the local
-    <code>shapepipe-dev</code> container on clean <code>ngmix_v2.0</code> too (container/ngmix version
-    skew, most likely) — CI on the pushed branch is the arbiter. The five other tests, including my two
-    new ones, pass against the branch code.</p>
+    <code>shapepipe-dev</code> container — reproduced by the review session on clean
+    <code>ngmix_v2.0</code> with the identical error (<code>ngmix.fitting</code> has no
+    <code>Fitter</code>: the local container carries an older ngmix than the branch expects). The
+    other tests, including the two new size tests, pass against the branch code. Note shapepipe's
+    <code>deploy-image.yml</code> triggers only on PRs/pushes targeting
+    <code>develop</code>/<code>main</code>/<code>master</code> — so a PR of this branch into
+    <code>ngmix_v2.0</code> won't run CI by itself; the suite gets arbitrated when the change rides
+    PR #740's chain toward <code>develop</code>.</p>
   </section>
 
   <hr class="short">