Analysis page: histogram drops the max value, Compare prints over 100% lower, and 13 other findings in analysis.py

[chvvkumar]

Found while reading backend/app/api/analysis.py and the Analysis page components line by line at 591234b, and by running the module's own functions over a real library (416 LIGHT frames, 3 nights, 2 rigs with plate scales of about 0.99 and 2.94 arcsec per pixel). Grouped by how visible the fault is.

Wrong output a user sees

1. The histogram drops the maximum value (analysis.py:542)

The last bin's upper edge is accumulated as v_min + i * bin_width + bin_width, which is usually not exactly v_max in binary floating point. When it lands below v_max, the test i == n_bins - 1 and v == b_end fails and the frame at the maximum falls in no bin.

Observed: v_min 0.0, v_max 2.6245, 10 bins, bin_width 0.26244999999999996, last edge 2.6244999999999994. Bin counts sum to 415 of 416 and the last bin reads 0 instead of 1.

Suggested fix: use v_max as the last bin's upper edge, or test i == n_bins - 1 and v >= b_start.

2. The Compare verdict prints percentages above 100 (analysis.py:890 to 896)

pct_diff divides by med_a whenever it is non-zero, but the sentence always names the smaller group as "X% lower" than the larger. When the smaller median is under half the larger, the sentence reads over 100 percent lower.

Observed on three of three real comparisons: two rigs, HFR in arcsec, 1.772 against 3.997, reads "126% lower" (actual 55.7%); two filters, 1.944 against 3.879, reads "100% lower"; an FWHM comparison reads "111% lower".

Suggested fix: divide by the larger of the two medians, which is the one the sentence compares against.

3. The correlation verdict assumes lower Y is always better (CorrelationChart.tsx:55 to 63)

A rising slope is called a "negative impact" and a falling one "improves". detected_stars and sky_quality are in the Y list and are higher-is-better, so the sentence is backwards for both. TimeSeriesTab.tsx:18 already keeps a HIGHER_IS_BETTER set that the correlation verdict could share.

4. The Time Series tooltip names an arbitrary target on a multi-target night (analysis.py:712 to 713)

tid_list = list(g["target_ids"]) then tnames.get(tid_list[0]) takes the first element of a set. On a night with two targets the label is one of them, chosen by hash order, and can differ between processes. Two of the three real nights carry two targets.

Suggested fix: show the name only when the night has one target, otherwise "Mixed" or the joined names.

5. Raw pixel HFR is plotted across rigs with different plate scales

_PIXEL_METRICS has one reader, get_compare (analysis.py:843). Correlation, distribution, box plot, time series and matrix plot median_hfr in pixels even when the filter spans two optical trains. On the library above, pixel HFR ranks the two rigs the opposite way from arcsec HFR (1.36 against 1.79 px; 4.00 against 1.77 arcsec). The existing "not comparable across optical trains" note is easy to miss. A visible warning when the selected frames span more than one arcsec_per_pixel value would help, or an arcsec option on those tabs.

6. Help text that does not match the page

• The Matrix popover describes a heatmap of one metric across two categorical axes ("integration time per channel", "telescope by filter"). The tab is a Pearson matrix over metric pairs.
• The Time Series popover says data is "either per frame or aggregated by session or night". The endpoint ignores granularity; so do box plot, matrix and compare. Only correlation and the histogram honour it, and nothing in the UI says so.
• Several popovers call the date range "capture date". _apply_filters filters session_date (analysis.py:167).

Statistical or consistency issues

7. The "95% CI" band is too narrow on small samples (analysis.py:305)

The multiplier is 1.96 if n > 30 else 2.0. The two-sided 95 percent t value at n = 4 is 4.30, so the band is under half its correct width there while still labelled "95% CI". A small t table, or relabelling the band as indicative, would fix the claim.

8. _pearson_r answers 0.0 for three different conditions (analysis.py:239 to 250)

n below 3, a constant X and a constant Y all return exactly 0.0, which is also a valid result for uncorrelated data. The page cannot distinguish "no correlation" from "not enough data" or "this column is constant". The matrix path uses PostgreSQL corr() (:791), which returns NULL for a constant column, so the two paths disagree on the same data. Returning None and rendering "n/a" would align them.

9. Box plot "By Month" groups by capture_date while every other grouping and the date filter use session_date (analysis.py:597 against :167)

A frame taken after midnight on the last night of a month lands in the next month's box while its session belongs to the previous one.

10. "Not enough data" behaves two ways on one page (analysis.py:529, :650 to 652, :883)

Distribution raises HTTP 400 under 2 values and compare raises 400 under 4 per group, while the box plot silently drops a group under 4. A user who filters down to 3 frames per filter sees an empty chart with no reason.

11. Skewness is computed from already-rounded inputs (analysis.py:546 to 550)

stats.mean and stats.std_dev come from _compute_summary_stats rounded to 6 decimals, then get cubed. Harmless in practice, but the last digits differ from a full-precision computation.

Performance

12. _is_outlier_iqr re-sorts both axes once per point (analysis.py:226 to 236, called at :446)

For n points this is 2n sorts of n elements. The quartiles do not depend on the point being tested, so they can be computed once outside the comprehension with identical flags. At the 100,000 frames the comment at :457 anticipates, this is 200,000 sorts of 100,000 elements per request.

Dead or unreachable code

13. Dead branch in /distribution (analysis.py:561 to 563)

_compute raises HTTPException rather than returning None, so the caller's if data is None check never fires.

14. The first "not comparable" sentence in Compare is unreachable

The backend sets both arcsec medians only inside the comparable branch (analysis.py:910 to 913), so comparable == false always arrives with both null, and the frontend's crossTrainVerdict branch that reads the two medians can never run. The backend's own verdict string for that case is built and then discarded by the frontend.

15. No ORDER BY on any statement feeding a figure

Row order is whatever PostgreSQL returns. The builtin float sum() results, which points survive the 5,000 point downsample, and the emission order of session groups all depend on it, so two identical requests can differ in the last digits or in which points are drawn. ORDER BY session_date, id would make results reproducible.

[chvvkumar]

All 15 findings are fixed on snd in 62c16c4.

Wrong output a user sees

1. Histogram: the last bin now ends exactly at v_max and is closed on the right, so bin counts always sum to the sample size (including the all-equal case).
2. Compare verdict: the percentage divides by the larger median, the one the sentence compares against. 1.772 against 3.997 now reads 56% lower.
3. Correlation verdict: wording follows each metric's polarity. HIGHER_IS_BETTER moved to a shared module used by both the Time Series and Correlation tabs. ADU mean, median and stdev get neutral increase/decrease wording. Correction to the report: of the two higher-is-better metrics only detected_stars is a Y metric; sky_quality is X-only.
4. Time Series tooltip: one target shows its name; several show the sorted names joined, capped at three plus "+N more".
5. Plate scales: the correlation, distribution, box plot, time series and matrix responses carry mixed_plate_scales, true when a pixel metric is plotted and the filtered frames span more than one arcsec_per_pixel value (rounded to 2 decimals). One shared warning component renders on all of those tabs and replaces the old note. Known limit: the check uses the equipment, filter and date filters only, so it can fire when the second plate scale belongs to frames that have no value for the plotted metric.
6. Help text: Matrix is described as a Pearson grid with the 10 paired frame minimum, Time Series as the nightly median, Granularity as applying to the Correlation tab and the Distributions histogram only, and the date range as imaging night. The Shared Filters text now also says that Compare uses the date range only.

Statistical or consistency issues

7. The confidence band uses the two-sided 95 percent Student t value for df = n - 2 (lookup table for df 1 to 30, 1.96 above). n = 4 gives 4.303.
8. _pearson_r and _spearman_rho return None for fewer than 3 points or a constant axis. The page prints "n/a", matching the matrix path.
9. Box plot "By Month" groups by session_date.
10. The box plot response lists groups dropped for having fewer than 4 frames, and the tab shows them under the chart, including when the chart is empty. The 400 detail from /distribution, /boxplot and /compare is now shown; all three tabs were swallowing it.
11. Skewness is computed from the full-precision mean and sample standard deviation.

Performance

12. IQR fences are computed once per axis per request. Flags are identical to the old per-point function, checked by a brute-force test.

Dead or unreachable code

13. The dead if data is None branch in /distribution is removed.
14. The unreachable arcsec branch in the Compare tab is removed. When the groups are not comparable the tab shows the backend's verdict string.
15. Every row-returning statement feeding a figure is ordered by session_date, id. /matrix (a single aggregate row) and the distinct plate-scale check are left unordered.

Verification

• Backend: 8 new pure tests in tests/test_analysis_stats.py plus the 23 existing mocked endpoint tests pass. The 5 test_correlation_phd2_* tests need a live Postgres and were not run.
• Frontend: tsc --noEmit is clean against regenerated types, and vitest passes (58 files, 746 tests).
• Not yet run against a database: the plate-scale query (round(CAST(arcsec_per_pixel AS NUMERIC), 2)) and the new ORDER BY were compile-checked against the PostgreSQL dialect only. One smoke request per Analysis tab after deploy covers this.

frontend/openapi.json and the generated types were stale, so the refresh in this commit also picks up the API key and mosaic from-sessions endpoints. Cached analysis responses expire after 300 seconds, so no cache key change was needed.