refactor(calibration): PR #71 review follow-ups — trust settings type, flag enums in models, config cross-validation #78

New issue

Open

opened 2026-09-24 22:31:51 +00:00 by gabogg · 0 comments

gabogg commented

2026-09-24 22:31:51 +00:00

Owner

Non-blocking findings from the round-3 review of PR #71 (comment 1733).

Code quality

The 11 trust thresholds travel together (SQL updates, repository defaults, schema, service): group them in a TrustSettings model. Deduplicating their defaults across DDL, repository, schema and the HTML inputs is #74's single-source-of-defaults work. Do them together.
score_cycle_flags still checks "FLAG_INGESTION_GAP" / "FLAG_INGESTION_STALLED" as strings. Add CalibrationAnomalyFlag.is_ingestion() next to is_data_quality() / is_low_activity().
AutomaticCalibrationSample uses trust_status: str and anomaly_flags: list[str]. Use CalibrationTrustStatus / CalibrationAnomalyFlag, and replace the new literal compares (row.trust_status != "TRUSTED", SQL 'AUTO_EXCLUDED').
The cycle-date derivation is copied between sync and async record_calibration_log.
database.py has three ALTER loops with two tuple shapes. Unify them.
multiplier_provenance defaults differ: COMPUTED in CREATE, LEGACY_UNKNOWN in ALTER (deliberate: upgraded rows are classified at startup). Add a comment at both sites.

Behaviour

No cross-field config validation: trust_ratio_min > trust_ratio_max is accepted, and trust_spike_min_samples > trust_spike_window silently disables the relative spike rule.
Existing databases keep computed_exit_multiplier DEFAULT 1.0 (no table rebuild). Latent: every insert passes the value explicitly today.

Acceptance criteria

Items 1–8 fixed or explicitly declined (item 1 coordinated with #74).
Full suite green.

Related: #34, #33, PR #71, #74.

> Non-blocking findings from the round-3 review of PR #71 (comment 1733). ## Code quality 1. **The 11 trust thresholds travel together** (SQL updates, repository defaults, schema, service): group them in a `TrustSettings` model. Deduplicating their defaults across DDL, repository, schema and the HTML inputs is **#74**'s single-source-of-defaults work. Do them together. 2. **`score_cycle_flags` still checks `"FLAG_INGESTION_GAP"` / `"FLAG_INGESTION_STALLED"` as strings.** Add `CalibrationAnomalyFlag.is_ingestion()` next to `is_data_quality()` / `is_low_activity()`. 3. **`AutomaticCalibrationSample`** uses `trust_status: str` and `anomaly_flags: list[str]`. Use `CalibrationTrustStatus` / `CalibrationAnomalyFlag`, and replace the new literal compares (`row.trust_status != "TRUSTED"`, SQL `'AUTO_EXCLUDED'`). 4. **The cycle-date derivation is copied** between sync and async `record_calibration_log`. 5. **`database.py` has three ALTER loops with two tuple shapes.** Unify them. 6. **`multiplier_provenance` defaults differ:** `COMPUTED` in `CREATE`, `LEGACY_UNKNOWN` in `ALTER` (deliberate: upgraded rows are classified at startup). Add a comment at both sites. ## Behaviour 7. **No cross-field config validation:** `trust_ratio_min > trust_ratio_max` is accepted, and `trust_spike_min_samples > trust_spike_window` silently disables the relative spike rule. 8. **Existing databases keep `computed_exit_multiplier DEFAULT 1.0`** (no table rebuild). Latent: every insert passes the value explicitly today. ## Acceptance criteria - [ ] Items 1–8 fixed or explicitly declined (item 1 coordinated with #74). - [ ] Full suite green. Related: #34, #33, PR #71, #74.

gabogg added the

ready-for-agent

label

2026-09-24 22:31:52 +00:00

gabogg referenced this issue

2026-09-24 22:32:35 +00:00

fix(calibration): calibration audit trail and trust vocabulary (#34, #33) #71