Code Review

End-to-End Code Review

Migrated from REVIEW_CODEX.md in the main repository.

Codex Review (2026-02-16)

Reviewer: Codex (GPT-5)

Reviewed repository structure and key runtime paths end-to-end. Attempted full test run, but execution was blocked by missing runtime dependencies.

Claude Review Pass (2026-02-15)

Reviewer: Claude Opus 4.6

Verified all 11 Codex findings against source code with line-level inspection. Identified additional issues not covered by original review.

Verdict on Codex Findings: 11/11 confirmed valid

Findings Summary (17 total, ordered by severity)

Critical Issues (3)

1. Plot pipeline is disconnected from validated config schema - Plot schema defines data: list[str] but runtime reads pairs instead. No mapping logic exists.

2. Requested statistics metrics can be silently ignored - Config has default stat_list that shadows user-supplied metrics, affecting all production configs.

3. ModelData.extract_surface() ignores CESM vertical convention - Always takes isel({level_dim: 0}) when should check if pressure increases with index.

High Priority (4)

4. Relative file paths resolved against process CWD, not config file directory.

5. Skipped stages not finalized in runner UI/log lifecycle — SKIPPED status leaves live animation running.

6. Time filtering has off-by-one bug for ISO format timestamps — regex doesn't match T separator.

7. time_tolerance is largely not enforced during pairing — all 5 strategies accept but none enforce it.

Medium Priority (6)

8. NME_% fallback formula in CSV summary is incorrect — uses RMSE instead of ME.

9. CLI --strict validation mode is not implemented — flag shown but not wired through.

10. Swath pairing collapses residual dimensions by averaging — should interpolate instead.

11. Statistics stage silently swallows per-pair computation errors — not logged to console.

12. Variable name collision risk in paired dataset assembly.

13. Pairing stage can report success even when all pairings fail — returns COMPLETED when paired_count == 0.

Low Priority (4)

14. Statistics summary CSV loses pair identity — no Pair column.

15. README example passes wrong config type to PipelineContext.

16. Inconsistent Dask worker configuration between stages.

17. compute_tropospheric_column() defined but never used — dead code.

Test Coverage Gaps

• No end-to-end tests for plotting using schema-native plots.data references
• No tests asserting --strict validation behavior
• No tests for time_tolerance enforcement across pairing strategies
• No tests for relative-path resolution from non-CWD locations
• No tests for ModelData.extract_surface() with CESM-convention vertical coordinates
• No tests for ISO-format (T separator) time filtering behavior

Overall Assessment

Strong structural organization and substantial test coverage (792 tests passing) around core primitives. Clean, well-decomposed architecture.

Most impactful issues are integration mismatches between config schema and runtime pipeline stages.

MODIS Review (2026-03-09)

Reviewer: Codex (GPT-5)

MODIS Findings (5 total)

M1) Critical: MODISL2AODReader does not match the real monetio MODIS API.

M2) High: The standard pipeline still has no MODIS-specific load-and-bin path.

M3) High: SwathGridStrategy is implemented but unreachable from pipeline.

M4) Medium: The proposed MODIS YAML contract not compatible with current runtime contract.

M5) Medium: Test coverage misses real MODIS integration path.