RFingAdam · RFingAdam · Aug 2, 2026 · Jul 31, 2026
diff --git a/docs/REVIEW_2026-05.md b/docs/REVIEW_2026-05.md
@@ -0,0 +1,147 @@
+# RFlect Multi-Perspective Review — 2026-05 (post-v5.0.0)
+
+A backend/architecture, RF-engineering, and frontend/UX/docs review of RFlect after the
+v5.0.0 zero-LLM relaunch. Findings are de-duplicated, severity-ranked, and each maps to a
+proposed GitHub issue under a version milestone (**v5.1** correctness & quick wins, **v5.2**
+RF analysis expansion, **v6.0** automation & platform) with a theme label
+(`rf-accuracy`, `rf-newmethod`, `automation`, `ux`, `tech-debt`, `test-coverage`, `documentation`).
+
+Method: three specialized agents reviewed in parallel; every high-severity correctness claim
+was then **independently adjudicated against the code and the relevant standard** before being
+recorded here. Where an agent over-flagged, that is stated explicitly.
+
+---
+
+## Headline adjudication: TRP normalization is CORRECT (agent over-flagged)
+
+The RF agent's top finding claimed `calculate_trp` (`plot_antenna/calculations.py:36`) divides
+by an extra `1/(4π)` and under-reports TRP by ~11 dB. **This was verified false.**
+
+The chamber per-angle input is **EIRP** (isotropic-referenced power after path-loss cal), and
+for EIRP the canonical CTIA/IEEE-149 discrete form **is**
+`TRP = (1/4π) Σ EIRP·sinθ·Δθ·Δφ`. Proof by the isotropic oracle: feeding a constant 0 dBm
+EIRP pattern returns **TRP = −0.003 dBm** (an 11 dB bug would return ≈ −11 dBm). The
+−0.003 dB residual also bounds the quadrature error (A2) as negligible at 5° spacing.
+
+The genuine, valuable takeaways from that thread are downgraded to: a **golden-reference
+regression test** to lock the result in (R8), and a **docstring clarification** that the input
+is EIRP, not "radiated power density" (R7). This is exactly why the review adjudicates before
+filing.
+
+---
+
+## Findings → proposed issues
+
+Severity: 🔴 high · 🟠 medium · 🟢 low. Effort: S/M/L.
+
+### Correctness (rf-accuracy)
+
+| ID | Finding | Sev | Effort | Milestone | Evidence |
+|----|---------|-----|--------|-----------|----------|
+| R1 | **Group-delay variance & std-dev are dead code** — advertised dispersion metrics never computed; `variance_at_freq` references an undefined name. | 🟠 | S | v5.1 | `groupdelay.py:158-166` |
+| R2 | **Sidelobe detector has no main-lobe guard** — local maxima inside the main beam can be mis-reported as the first sidelobe (no ≥HPBW exclusion from boresight). | 🟠 | S | v5.2 | `analysis_engine.py:308-329` |
+| R3 | **Rician fade margin uses a dB-Gaussian approximation** — no mean shift; misstates deep-fade margin 1–3 dB at low/moderate K. Fine for estimation, not compliance. | 🟠 | M | v5.2 | `calculations.py:1437,1455+` |
+| R4 | **TRP quadrature is rectangular** (no pole/φ-wrap half-cells). Verified negligible (~0.003 dB at 5°, grows at coarse grids). Optional precision polish. | 🟢 | S | v6.0 | `calculations.py:30,36` |
+
+*(TRP 1/4π normalization, ECC, Vaughan-Andersen DG, Kraus directivity, HPBW walk, Friis/log-distance/ITU path loss — all audited and confirmed CORRECT.)*
+
+### MCP tool-contract consistency (tech-debt)
+
+| ID | Finding | Sev | Effort | Milestone | Evidence |
+|----|---------|-----|--------|-----------|----------|
+| R5 | **`cal_drift_report` can raise to the client** — `raise ValueError` on unknown format breaks the universal "never raise; return warnings" tool contract. | 🔴 | S | v5.1 | `rflect-mcp/tools/cal_drift_tools.py:96` |
+| R6 | **`bulk_tools` + `analysis_tools` return bare strings on error**, not the `{...,"warnings":[]}` dict contract — agents can't programmatically detect failure. | 🟠 | M | v5.1 | `bulk_tools.py:108,170,213,290`; `analysis_tools.py:152,212,265,365,446` |
+
+### Documentation & correctness-guarding
+
+| ID | Finding | Sev | Effort | Milestone | Evidence |
+|----|---------|-----|--------|-----------|----------|
+| R7 | **Clarify TRP docstring** — input is EIRP (isotropic-referenced), not "radiated power density"; the 1/4π is the CTIA-EIRP form. Prevents future false-alarm "bugs." | 🟢 | S | v5.1 | `calculations.py:8-25` |
+| R8 | **Golden-reference regression test for TRP/gain** — lock isotropic→0 dB + a known-pattern fixture with numeric tolerance in CI. Highest-leverage safeguard for the analysis core. | 🔴 | M | v5.1 | `tests/` (none today) |
+
+### Test coverage (test-coverage)
+
+| ID | Finding | Sev | Effort | Milestone | Evidence |
+|----|---------|-----|--------|-----------|----------|
+| R9 | **`plotting.py` (3710 lines, 26 plot fns) has zero tests** — called from both GUI and MCP report path; a regression silently produces wrong plots. Start with pure-compute fns. | 🟠 | L | v5.1 | no `tests/test_plotting.py` |
+| R10 | **`save.py`, `groupdelay.py`, `plot_group_delay_cst.py`, `uwb_plotting.py` untested** — extract I/O from compute and unit-test the pure functions. | 🟠 | M | v5.1 | `tests/` |
+
+### Architecture / tech-debt (tech-debt)
+
+| ID | Finding | Sev | Effort | Milestone | Evidence |
+|----|---------|-----|--------|-----------|----------|
+| R11 | **DOCX builder duplicated** across GUI `save.py` and MCP `report_tools.py` (near-verbatim `_add_gain_stats_table`, `add_branded_heading`, brand init) — already drifting. Extract `plot_antenna/docx_helpers.py`. | 🟠 | M | v6.0 | `save.py:420-537`; `report_tools.py:1221-1268` |
+| R12 | **`generate_advanced_analysis_plots` is a 27-parameter god-function** spanning 5 analysis domains — use per-domain dataclass configs (mirror `ReportOptions`). | 🟠 | L | v6.0 | `plotting.py:3568` |
+| R13 | **Refactor the giant modules** — `plotting.py` (3710), `report_tools.py` (2017), `calculations.py` (1949), `file_utils.py` (1665) into cohesive submodules. | 🟢 | L | v6.0 | (sizes) |
+| R14 | **Silent `except…pass`** in GUI swallows settings-save / parse failures with no log. ~8 sites; route through `log_message`. | 🟠 | S | v5.1 | `main_window.py:825,855,878`; `gui/utils.py:41,72`; `callbacks_mixin.py:304,1261,1286` |
+
+### Packaging / dependencies (tech-debt)
+
+| ID | Finding | Sev | Effort | Milestone | Evidence |
+|----|---------|-----|--------|-----------|----------|
+| R15 | **`openai`/`keyring`/`cryptography` still core deps after AI removal** — imported nowhere (verified); ~50 MB of dead transitive deps. Drop or move `openai` to an `ai` extra. | 🟠 | S | v5.1 | `pyproject.toml:36-42`, `requirements.txt:11-25` |
+| R16 | **`pyinstaller` in runtime `requirements.txt`** — build-only tool installed for end users/CI; already in the `exe` extra. Remove from runtime. | 🟠 | S | v5.1 | `requirements.txt:27-29` |
+| R17 | **`numpy<2.0.0` ceiling** blocks environments needing numpy 2.x (stable since 2024). Test + raise to `<3.0.0`. | 🟢 | S | v5.1 | `pyproject.toml:31` |
+| R18 | **`rflect-mcp` not in the installable package** — needs a separate manual install; document prominently or make it an `mcp` extra/workspace package. | 🟢 | M | v6.0 | `pyproject.toml:73` |
+
+### Frontend / UX (ux)
+
+| ID | Finding | Sev | Effort | Milestone | Evidence |
+|----|---------|-----|--------|-----------|----------|
+| R19 | **Main plot generation blocks the Tk thread** — 26 plots built synchronously via `after()`, progress bar can't animate, app appears frozen on big scans. Elegant fix: render with Agg off-thread, display saved PNGs. | 🔴 | L | v6.0 | `callbacks_mixin.py:1236-1273` |
+| R20 | **Report generation blocks the UI** with no progress/thread (bulk processing already does it right). Reuse the worker+progress pattern. | 🟠 | M | v5.2 | `tools_mixin.py:275-284` |
+| R21 | **Errors only `print()` to stdout / silent** — invisible in a packaged GUI; route to `log_message`/`messagebox`. | 🟠 | S | v5.1 | `callbacks_mixin.py:1233`; `main_window.py` prints |
+| R22 | **Plot styling duplicated across 26 functions** — `dpi=300` ×24, 5+ colormaps, mixed save formats; add `_apply_style()`/`_save_fig()` helpers. | 🟠 | M | v6.0 | `plotting.py` |
+| R23 | **Hardcoded `Arial`/`Segoe UI` fonts + raw `tk.*` dialogs ignore dark theme** — break layout/theming on Linux. Use font constants + ttk. | 🟢 | M | v5.2 | `gui/utils.py:144,152`; `callbacks_mixin.py:371+` |
+| R24 | **Determinate progress + dialog keyboard nav** — bulk bar is indeterminate despite known job count; dialogs lack Return/Escape. | 🟢 | S | v5.2 | `tools_mixin.py:464,610` |
+
+### Documentation (documentation)
+
+| ID | Finding | Sev | Effort | Milestone | Evidence |
+|----|---------|-----|--------|-----------|----------|
+| R25 | **Thin reference docs** — `glossary.md`/`measurement-types.md`/`mcp/overview.md` are stubs. (Glossary partly expanded in v5.0.0; finish coverage of every emitted metric.) | 🟢 | M | v5.1 | `docs/reference/*` |
+| R26 | **Low example-plot coverage** — `polarization`, `uwb-sff`, `maritime-horizon`, `advanced-rf`, `cal-drift` user-guide pages have no output screenshots. | 🟢 | M | v5.2 | `docs/user-guide/*` |
+| R27 | **No CHANGELOG** — update-check logic exists but no changelog page; add Keep-a-Changelog (RELEASE_NOTES.md can seed it). | 🟢 | S | v5.1 | repo root |
+
+---
+
+## RF-platform roadmap (rf-newmethod / automation — aggressive scope)
+
+| ID | Capability | Value | Effort | Milestone | Theme |
+|----|-----------|-------|--------|-----------|-------|
+| P1 | **Automated PASS/FAIL vs spec limit-lines** in reports (min gain, max ECC, TRP floor, SLL ceiling). Pairs with P2. | 🔴 | S | v5.2 | rf-newmethod |
+| P2 | **Regulatory spec-mask checks** — FCC Part 15 (§15.247/§15.407) + ETSI EN 300 328/301 893 EIRP & spurious masks. Wire in the `emc-regulations` MCP. | 🔴 | M | v5.2 | rf-newmethod |
+| P3 | **Measurement-uncertainty budgets / error bars** on TRP & gain (mismatch, cable drift, positioner, ref-antenna) — CTIA-style expanded-uncertainty table. | 🔴 | L | v5.2 | rf-accuracy |
+| P4 | **CTIA OTA TIS/TRP test-plan templates** — TIS (Rx sensitivity) path is entirely missing; add CTIA grid/partial-sphere templates. | 🔴 | L | v6.0 | rf-newmethod |
+| P5 | **Multiport Touchstone .s3p/.s4p + mixed-mode S-params** (Sdd/Scc/Sdc) for differential/MIMO feeds. | 🟠 | M | v5.2 | rf-newmethod |
+| P6 | **S-parameter time-gating + de-embedding** (IFFT gate chamber reflections; TRL/port-extension). | 🟠 | L | v6.0 | rf-newmethod |
+| P7 | **Statistical pattern averaging across repeat measurements** — per-point mean/σ + repeatability (complements cal-drift). | 🟠 | M | v5.2 | rf-accuracy |
+| P8 | **SCPI instrument control** (pyvisa) — live VNA/signal-analyzer sweep setup + trace pull; closes the file-only loop. | 🔴 | L | v6.0 | automation |
+| P9 | **Chamber positioner automation** (θ/φ turntable driver); with P8 enables unattended full-sphere scans. | 🟠 | L | v6.0 | automation |
+| P10 | **Scheduled/automated cal-drift monitoring** — cron-driven ingest + threshold alerts (extends the existing cal-drift framework + issues #3–#6). | 🟠 | S | v5.2 | automation |
+| P11 | **n-antenna comparison tables** (Min/Max VSWR/Eff/Gain) — finishes existing issue #7 on top of `compare_antennas`. | 🟠 | M | v5.2 | rf-newmethod |
+| P12 | **Phased-array / beam-steering synthesis** (element × array factor, taper, steering, grating-lobe check). | 🟢 | L | v6.0 | rf-newmethod |
+| P13 | **Near-field-to-far-field transform** (spherical/planar modal expansion) — extend beyond far-field ranges. | 🟢 | L | v6.0 | rf-newmethod |
+| P14 | **Circular-polarization axial-ratio + 3-antenna gain-comparison method** — expected on a full OTA bench. | 🟠 | M | v6.0 | rf-newmethod |
+
+---
+
+## Existing-issue reconciliation
+
+| Issue | Disposition |
+|-------|-------------|
+| #1 "thin fallback exec summary" | **Close** — superseded by the v5.0.0 deterministic summary + agent-`narrative` parameter. |
+| #2 "cross-file overlay comparison tool" | **Close** — delivered as `compare_antennas` in v5.0.0. |
+| #3/#4/#5/#6 cal-drift (recert reminder, threshold alerts, cable-loss .s2p history, passive cal) | **Keep** — fold under P10 / the `automation` theme; assign to v5.2. |
+| #7 "n-antenna VSWR/Eff/Gain tables" | **Re-scope** — partially met by `compare_antennas`; remaining tabular work tracked as P11 (v5.2). |
+
+---
+
+## Milestone roll-up
+
+- **v5.1 — Correctness & quick wins:** R1, R5, R6, R7, R8, R9, R10, R14, R15, R16, R17, R21, R25, R27 + close #1/#2.
+- **v5.2 — RF analysis expansion:** R2, R3, R20, R23, R24, R26, P1, P2, P3, P5, P7, P10, P11 + #3–#6.
+- **v6.0 — Automation & platform:** R4, R11, R12, R13, R18, R19, R22, P4, P6, P8, P9, P12, P13, P14.
+
+No code behavior was changed by this review. Each row above becomes a GitHub issue with
+acceptance criteria and `path:line` links once approved.