gabogg/hikcentral
Watch
1
0
Fork
You've already forked hikcentral
 0
Code Issues 113 Pull requests 8 Projects Releases Packages Wiki Activity Actions

fix(calibration): calibration honesty — seeded k from config, open-window dwell (#36, #32) #70

Merged
gabogg merged 14 commits from fix/calibration-honesty into master 2026-09-24 22:33:07 +00:00
Conversation 7 Commits 14 Files changed 16 +599 -193
gabogg commented 2026-09-23 22:49:06 +00:00
Owner
Copy link

Closes #36
Closes #32

Implemented and ready for review.

Problem

#36: an uncalibrated deployment publishes a borrowed multiplier. k = 1.1162, a result measured at this mall, is the hardcoded fallback in compute_ewma_multiplier (occupancy_service.py:877-878), refresh_active_multiplier_async (:919-920), the config reads (:1031, :1572) and the repository defaults (occupancy_repository.py:91, :107, :133, :179). With no trusted history, a new deployment shows "calibrated" figures scaled by another site's constant. The default variance 0.0002 (:856, :896-902, :930, repo :93, :109, :135) draws the narrowest CI band exactly when the sample is empty.

#32: Mean Dwell is inflated. get_cycle_average_occupancy_async (occupancy_repository.py:1169) integrates occupancy over the full 24 h cycle, and calculate_proportional_occupancy (occupancy_models.py:75) floors it at the patrol guard count (max(N_patrol, round(I − k·E + N_patrol))). Closed hours add guards × ~10 h, and the clamp hides any k over-estimate. The code also contradicts CONTEXT.md's O(t) = max(0, E − X_adj + offset).

Approach (settled)

#36

  • Seed k from occupancy_config.initial_exit_multiplier from day one: this deployment 1.1162, a fresh deployment 1.0. Remove the scattered 1.1162 literals.
  • UNCALIBRATED / SIN CALIBRAR chip on calibrated tiles until 14 trusted business cycles.
  • Default variance when N < 2 derived from the guardrail: SD ≈ (1.35 − 0.90) / 4 ≈ 0.11.
  • Guardrail stays [0.90, 1.35]. Already done: CONTEXT.md was corrected in #63.

#32

  • Ō / W_mean over open_time → close_time inside the configured accounting cycle.
  • Occupancy floored at 0: drop the daytime +N_patrol offset and the max(N_patrol, …) clamp, matching CONTEXT.md. patrol_guard_count becomes calibration-only.
  • 3 open-window dayparts: equal-duration by default, volume-tercile as an option; clock-labelled; low-sample flag; day and week views.
  • The KPI labels its own window (94 min · 10:00–22:00).

Shared: ADR "Published occupancy figures — measurement window & calibration honesty" (dwell redefinition, the historical-discontinuity trade-off, the calibration-honesty policy). CONTEXT.md: open-window dwell, dayparts, uncalibrated state.

Implementation notes

The database migration preserves an existing site's active multiplier as its seed; a fresh database starts at 1.0. The API supplies maturity, open-window dwell and dayparts. The current calibration desk shows the uncalibrated state; Statistics Deck presentation remains assigned to PR #20. The ADR records the historical dwell cutover.

Acceptance criteria

#36

  • Seed k from occupancy_config.initial_exit_multiplier; no hardcoded 1.1162 in function signatures or repository defaults.
  • Fresh-deployment config defaults to 1.0; this deployment's config carries 1.1162.
  • Maturity state exposed until 14 trusted business cycles (chip rendered by the deck, see point 1).
  • Default variance when N < 2 derived from the guardrail (SD ≈ 0.11).
  • Guardrail stays [0.90, 1.35]; CONTEXT.md matches (done in #63).
  • Tests: a fresh deployment applies k = 1.0; an uncalibrated one reports the uncalibrated state and a wide variance.

#32

  • Ō / W_mean over open_time → close_time within the configured cycle.
  • Occupancy floored at 0; +N_patrol daytime offset removed; code matches CONTEXT.md.
  • patrol_guard_count used only for nocturnal calibration convergence.
  • 3 dayparts (equal-duration default, volume-tercile option), clock-labelled, low-sample flag.
  • Tests: closed hours no longer inflate Ō; a slightly high k can drive occupancy below the old guard floor.

Shared

  • ADR authored; CONTEXT.md gains open-window dwell, dayparts, uncalibrated state.
  • Full suite green.

Sequencing across the [data-veracity] drafts

Declared together on 2026-09-23; each one is triaged, then reviewed, then implemented, in this order:

  1. #68: ingestion honesty (#26, #31, #30). Owns the sync loop and the peak walk.
  2. #69: business cycles in facility time (#27). Introduces the cycle-boundary seam that #32 and #34 then use.
  3. #70: calibration honesty (#36, #32). Changes the occupancy formula that #30's peak walk (in #68) calls.
  4. #71: calibration audit trail and trust vocabulary (#34, #33). Builds on #31's FLAG_INGESTION_GAP (in #68) and on #27's seam for cycle_date.

Review the PRs in this order; no PR has been merged into master.

🤖 Generated with Claude Code

Implementation decisions

Recorded after review round 2, which flagged these as scope creep:

  • occupancy_definition_cutover_at: records when the Mean Dwell / occupancy-floor definition changed (open point 3, historical discontinuity), so charts and the ADR can mark the cutover instead of comparing across it.
  • Dwell window metadata (dwell_window_start_epoch, dwell_window_end_epoch, dwell_window_label, dwell_kpi_label): implement #32's "the KPI self-labels its window (94 min · 10:00–22:00)".
Closes #36 Closes #32 **Implemented and ready for review.** --- ## Problem **#36: an uncalibrated deployment publishes a borrowed multiplier.** `k = 1.1162`, a result measured at *this* mall, is the hardcoded fallback in `compute_ewma_multiplier` (`occupancy_service.py:877-878`), `refresh_active_multiplier_async` (`:919-920`), the config reads (`:1031`, `:1572`) and the repository defaults (`occupancy_repository.py:91`, `:107`, `:133`, `:179`). With no trusted history, a new deployment shows "calibrated" figures scaled by another site's constant. The default variance `0.0002` (`:856`, `:896-902`, `:930`, repo `:93`, `:109`, `:135`) draws the **narrowest** CI band exactly when the sample is empty. **#32: Mean Dwell is inflated.** `get_cycle_average_occupancy_async` (`occupancy_repository.py:1169`) integrates occupancy over the full 24 h cycle, and `calculate_proportional_occupancy` (`occupancy_models.py:75`) floors it at the patrol guard count (`max(N_patrol, round(I − k·E + N_patrol))`). Closed hours add `guards × ~10 h`, and the clamp hides any `k` over-estimate. The code also contradicts `CONTEXT.md`'s `O(t) = max(0, E − X_adj + offset)`. ## Approach (settled) **#36** - Seed `k` from **`occupancy_config.initial_exit_multiplier`** from day one: this deployment `1.1162`, a fresh deployment `1.0`. Remove the scattered `1.1162` literals. - **`UNCALIBRATED` / `SIN CALIBRAR`** chip on calibrated tiles until **14 trusted business cycles**. - Default variance when `N < 2` derived from the guardrail: SD ≈ `(1.35 − 0.90) / 4 ≈ 0.11`. - Guardrail stays `[0.90, 1.35]`. **Already done:** `CONTEXT.md` was corrected in #63. **#32** - `Ō` / `W_mean` over **`open_time → close_time`** inside the configured accounting cycle. - Occupancy floored at **0**: drop the daytime `+N_patrol` offset and the `max(N_patrol, …)` clamp, matching `CONTEXT.md`. `patrol_guard_count` becomes calibration-only. - **3 open-window dayparts**: equal-duration by default, volume-tercile as an option; clock-labelled; low-sample flag; day and week views. - The KPI labels its own window (`94 min · 10:00–22:00`). **Shared:** ADR *"Published occupancy figures — measurement window & calibration honesty"* (dwell redefinition, the historical-discontinuity trade-off, the calibration-honesty policy). `CONTEXT.md`: open-window dwell, dayparts, uncalibrated state. ## Implementation notes The database migration preserves an existing site's active multiplier as its seed; a fresh database starts at 1.0. The API supplies maturity, open-window dwell and dayparts. The current calibration desk shows the uncalibrated state; Statistics Deck presentation remains assigned to PR #20. The ADR records the historical dwell cutover. ## Acceptance criteria **#36** - [x] Seed `k` from `occupancy_config.initial_exit_multiplier`; no hardcoded `1.1162` in function signatures or repository defaults. - [x] Fresh-deployment config defaults to `1.0`; this deployment's config carries `1.1162`. - [x] Maturity state exposed until 14 trusted business cycles (chip rendered by the deck, see point 1). - [x] Default variance when `N < 2` derived from the guardrail (SD ≈ 0.11). - [x] Guardrail stays `[0.90, 1.35]`; `CONTEXT.md` matches (done in #63). - [x] Tests: a fresh deployment applies `k = 1.0`; an uncalibrated one reports the uncalibrated state and a wide variance. **#32** - [x] `Ō` / `W_mean` over `open_time → close_time` within the configured cycle. - [x] Occupancy floored at 0; `+N_patrol` daytime offset removed; code matches `CONTEXT.md`. - [x] `patrol_guard_count` used only for nocturnal calibration convergence. - [x] 3 dayparts (equal-duration default, volume-tercile option), clock-labelled, low-sample flag. - [x] Tests: closed hours no longer inflate `Ō`; a slightly high `k` can drive occupancy below the old guard floor. **Shared** - [x] ADR authored; `CONTEXT.md` gains open-window dwell, dayparts, uncalibrated state. - [x] Full suite green. ## Sequencing across the `[data-veracity]` drafts Declared together on 2026-09-23; each one is triaged, then reviewed, then implemented, in this order: 1. **#68**: ingestion honesty (#26, #31, #30). Owns the sync loop and the peak walk. 2. **#69**: business cycles in facility time (#27). Introduces the cycle-boundary seam that #32 and #34 then use. 3. **#70**: calibration honesty (#36, #32). Changes the occupancy formula that #30's peak walk (in #68) calls. 4. **#71**: calibration audit trail and trust vocabulary (#34, #33). Builds on #31's `FLAG_INGESTION_GAP` (in #68) and on #27's seam for `cycle_date`. Review the PRs in this order; no PR has been merged into master. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ## Implementation decisions Recorded after review round 2, which flagged these as scope creep: - **`occupancy_definition_cutover_at`:** records when the Mean Dwell / occupancy-floor definition changed (open point 3, historical discontinuity), so charts and the ADR can mark the cutover instead of comparing across it. - **Dwell window metadata** (`dwell_window_start_epoch`, `dwell_window_end_epoch`, `dwell_window_label`, `dwell_kpi_label`): implement #32's "the KPI self-labels its window (`94 min · 10:00–22:00`)".
chore(wip): open draft for calibration honesty: seeded k, open-window dwell (#36, #32)
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m15s
Details
0025f3f3bc 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(calibration): seed site multiplier and measure open-window dwell
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m17s
Details
3221435faa
gabogg changed title from WIP: fix(calibration): calibration honesty — seeded k from config, open-window dwell (#36, #32) to fix(calibration): calibration honesty — seeded k from config, open-window dwell (#36, #32) 2026-09-24 14:25:36 +00:00
gabogg commented 2026-09-24 14:25:38 +00:00
Author
Owner
Copy link

Implementation is pushed at 3221435. This branch includes the preceding ingestion and facility-time commits for its tests. Review #68 and #69 first. The API and current calibration desk expose maturity; the Statistics Deck chip, confidence band and window presentation remain assigned to PR #20. No PR has been merged into master.

Implementation is pushed at `3221435`. This branch includes the preceding ingestion and facility-time commits for its tests. Review #68 and #69 first. The API and current calibration desk expose maturity; the Statistics Deck chip, confidence band and window presentation remain assigned to PR #20. No PR has been merged into `master`.
gabogg commented 2026-09-24 14:45:28 +00:00
Author
Owner
Copy link

Standards

Documented Standard Violations (Hard)

  • app/controllers/analytics_controller.py:27 & app/services/analytics_service.py:101:
    • Standard: docs/standards/code-standards.md §2.3 & AGENTS.md §2 (Pydantic Schemas: Use Pydantic v2 schemas in app/schemas/ for all API request bodies and structured data payloads. Avoid passing untyped, raw dictionaries between service layers).
    • Violation: Endpoint GET /dwell/dayparts and service method get_dwell_dayparts_async return untyped dict[str, Any] with nested dictionaries (days, dayparts), bypassing Pydantic response modeling.

Baseline Smells (Judgement Calls)

  • Speculative Generality (Dead Parameters) (app/schemas/occupancy_models.py:79, app/services/occupancy_service.py): patrol_guard_count: int = 8 is retained across calculate_proportional_occupancy, calculate_occupancy, and calculate_confidence_interval "for migration callers", but is completely unused in the logic following the zero-floor refactor.
  • Duplicated Code & Shotgun Surgery:
    • The magic literal 0.01265625 (0.1125^2) and seed default 1.0 are copy-pasted across 5 files in all three layers (database.py, occupancy_repository.py, occupancy_models.py, occupancy_service.py, analytics_service.py) rather than referencing a single domain constant in occupancy_models.py or config.py.
    • Little's Law dwell calculation (W = \bar{L} / \lambda) in analytics_service.py duplicates dwell calculation logic already encapsulated in occupancy_service.py.
  • Feature Envy (app/services/analytics_service.py): get_dwell_dayparts_async reaches directly into occ_mgr and occ_repo to slice cycle bounds, fetch raw event ingress, and compute dwell times, rather than delegating domain calculation to occupancy_service.py.
  • Mysterious Name (app/services/analytics_service.py): Magic constants low_sample = count_in < 30 and total >= 3 lack explanatory domain constants.

Spec

Missing or Partial Requirements

  • Frontend UNCALIBRATED State Chip (#36): Spec states: "- [x] Maturity state exposed until 14 trusted business cycles (chip rendered by the deck, see point 1)" and Issue #36: "when trusted_days_count < 7, the calibrated tiles should carry an UNCALIBRATED state chip". OccupancyCalibrationInfo exposes is_uncalibrated: bool via the API, but zero frontend templates or scripts were updated in this PR (deferred to PR #20).
  • Residual Dead Parameters in Signatures (#32): Spec states: "patrol_guard_count used only for nocturnal calibration convergence". patrol_guard_count was deactivated from daytime occupancy math, but remains in the function signatures of calculate_proportional_occupancy, compute_confidence_interval, and repository cycle queries as dead arguments.
  • Minimalist ADR Document: Spec states: "ADR 'Published occupancy figures — measurement window & calibration honesty'". 0006-published-occupancy-window-and-calibration-honesty.md is a 9-line prose document lacking standard ADR sections (Context, Decision Drivers, Considered Options, Consequences) and YAML frontmatter.

Scope Creep (Unasked Behaviour)

  • occupancy_definition_cutover_at DB Column & API Fields: Added a new database column and migration in database.py plus schema fields across OccupancyCalibrationInfo, /dwell/dayparts, and hourly timeseries to track definition cutover timestamp, which was not requested in #32 or #36.
  • Additional Dwell Window Fields in OccupancyLiveResponse: Expanded OccupancyLiveResponse with 4 metadata fields (dwell_window_start_epoch, dwell_window_end_epoch, dwell_window_label, dwell_kpi_label).
  • Hardcoded Magic Constant for Low-Sample Flag: Introduced count_in < 30 in analytics_service.py:94 without configuration or specification.

Incorrect Implementations

  • Volume Tercile Boundary Collapse: Spec states: "3 open-window dayparts: equal-duration default, volume-tercile option". In analytics_service.py:68-74, if a burst event advances accumulated ingress across both terciles simultaneously, identical timestamps are appended (cuts[0] == cuts[1]), causing the cuts[0] < cuts[1] validation check to fail and silently discarding volume slicing in favor of equal slicing. Slicing also fails if cuts[0] == open_epoch.
  • Hourly Timeseries Patrol Floor Residual: Spec states: "Occupancy floored at 0; +N_patrol daytime offset removed". In analytics_service.py:200-202, bucket data structures pre-initialize cumulative_occupancy and margins to patrol_guards instead of 0.

Summary: 5 standards findings (worst: untyped dict[str, Any] on /dwell/dayparts and shotgun surgery of variance literal 0.01265625); 8 spec findings (worst: volume tercile slicing silently falling back to equal duration during ingress bursts, and bucket pre-initialization retaining patrol guard offset).

## Standards ### Documented Standard Violations (Hard) - **`app/controllers/analytics_controller.py:27` & `app/services/analytics_service.py:101`**: - Standard: `docs/standards/code-standards.md` §2.3 & `AGENTS.md` §2 (*Pydantic Schemas: Use Pydantic v2 schemas in `app/schemas/` for all API request bodies and structured data payloads. Avoid passing untyped, raw dictionaries between service layers*). - Violation: Endpoint `GET /dwell/dayparts` and service method `get_dwell_dayparts_async` return untyped `dict[str, Any]` with nested dictionaries (`days`, `dayparts`), bypassing Pydantic response modeling. ### Baseline Smells (Judgement Calls) - **Speculative Generality (Dead Parameters)** (`app/schemas/occupancy_models.py:79`, `app/services/occupancy_service.py`): `patrol_guard_count: int = 8` is retained across `calculate_proportional_occupancy`, `calculate_occupancy`, and `calculate_confidence_interval` "for migration callers", but is completely unused in the logic following the zero-floor refactor. - **Duplicated Code & Shotgun Surgery**: - The magic literal `0.01265625` ($0.1125^2$) and seed default `1.0` are copy-pasted across 5 files in all three layers (`database.py`, `occupancy_repository.py`, `occupancy_models.py`, `occupancy_service.py`, `analytics_service.py`) rather than referencing a single domain constant in `occupancy_models.py` or `config.py`. - Little's Law dwell calculation ($W = \bar{L} / \lambda$) in `analytics_service.py` duplicates dwell calculation logic already encapsulated in `occupancy_service.py`. - **Feature Envy** (`app/services/analytics_service.py`): `get_dwell_dayparts_async` reaches directly into `occ_mgr` and `occ_repo` to slice cycle bounds, fetch raw event ingress, and compute dwell times, rather than delegating domain calculation to `occupancy_service.py`. - **Mysterious Name** (`app/services/analytics_service.py`): Magic constants `low_sample = count_in < 30` and `total >= 3` lack explanatory domain constants. ## Spec ### Missing or Partial Requirements - **Frontend UNCALIBRATED State Chip (#36)**: Spec states: `"- [x] Maturity state exposed until 14 trusted business cycles (chip rendered by the deck, see point 1)"` and Issue #36: *"when trusted_days_count < 7, the calibrated tiles should carry an UNCALIBRATED state chip"*. `OccupancyCalibrationInfo` exposes `is_uncalibrated: bool` via the API, but zero frontend templates or scripts were updated in this PR (deferred to PR #20). - **Residual Dead Parameters in Signatures (#32)**: Spec states: *"patrol_guard_count used only for nocturnal calibration convergence"*. `patrol_guard_count` was deactivated from daytime occupancy math, but remains in the function signatures of `calculate_proportional_occupancy`, `compute_confidence_interval`, and repository cycle queries as dead arguments. - **Minimalist ADR Document**: Spec states: *"ADR 'Published occupancy figures — measurement window & calibration honesty'"*. `0006-published-occupancy-window-and-calibration-honesty.md` is a 9-line prose document lacking standard ADR sections (Context, Decision Drivers, Considered Options, Consequences) and YAML frontmatter. ### Scope Creep (Unasked Behaviour) - **`occupancy_definition_cutover_at` DB Column & API Fields**: Added a new database column and migration in `database.py` plus schema fields across `OccupancyCalibrationInfo`, `/dwell/dayparts`, and hourly timeseries to track definition cutover timestamp, which was not requested in #32 or #36. - **Additional Dwell Window Fields in `OccupancyLiveResponse`**: Expanded `OccupancyLiveResponse` with 4 metadata fields (`dwell_window_start_epoch`, `dwell_window_end_epoch`, `dwell_window_label`, `dwell_kpi_label`). - **Hardcoded Magic Constant for Low-Sample Flag**: Introduced `count_in < 30` in `analytics_service.py:94` without configuration or specification. ### Incorrect Implementations - **Volume Tercile Boundary Collapse**: Spec states: *"3 open-window dayparts: equal-duration default, volume-tercile option"*. In `analytics_service.py:68-74`, if a burst event advances accumulated ingress across both terciles simultaneously, identical timestamps are appended (`cuts[0] == cuts[1]`), causing the `cuts[0] < cuts[1]` validation check to fail and silently discarding volume slicing in favor of equal slicing. Slicing also fails if `cuts[0] == open_epoch`. - **Hourly Timeseries Patrol Floor Residual**: Spec states: *"Occupancy floored at 0; +N_patrol daytime offset removed"*. In `analytics_service.py:200-202`, bucket data structures pre-initialize `cumulative_occupancy` and margins to `patrol_guards` instead of `0`. **Summary**: 5 standards findings (worst: untyped `dict[str, Any]` on `/dwell/dayparts` and shotgun surgery of variance literal `0.01265625`); 8 spec findings (worst: volume tercile slicing silently falling back to equal duration during ingress bursts, and bucket pre-initialization retaining patrol guard offset).
test: isolate background polling from API database tests
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m17s
Details
3da2ec0209
gabogg commented 2026-09-24 15:49:12 +00:00
Author
Owner
Copy link

Addressed the review in 981e8df and 3da2ec0 (including the earlier PR fixes).

  • Daypart output now has a typed response. The volume option interpolates boundaries so three nonempty intervals are produced, and low-sample detection has a named threshold.
  • Time-series buckets start at zero rather than at a patrol floor. The current dashboard shows an UNCALIBRATED chip, and the ADR now records the model and tradeoffs in full.
  • The full suite passes: 256 tests; the JavaScript suites passed as well.

The responsive statistics deck and its CI presentation belong to PR #20, as the description states; this PR exposes the maturity state and renders it on the current dashboard. The pre-existing patrol parameters and duplicated constants are cleanup candidates outside this review fix. No merge was performed.

Addressed the review in `981e8df` and `3da2ec0` (including the earlier PR fixes). - Daypart output now has a typed response. The volume option interpolates boundaries so three nonempty intervals are produced, and low-sample detection has a named threshold. - Time-series buckets start at zero rather than at a patrol floor. The current dashboard shows an `UNCALIBRATED` chip, and the ADR now records the model and tradeoffs in full. - The full suite passes: 256 tests; the JavaScript suites passed as well. The responsive statistics deck and its CI presentation belong to PR #20, as the description states; this PR exposes the maturity state and renders it on the current dashboard. The pre-existing patrol parameters and duplicated constants are cleanup candidates outside this review fix. No merge was performed.
gabogg commented 2026-09-24 16:07:59 +00:00
Author
Owner
Copy link

Review Follow-up & Verification of Previous Findings

Previous Review Status

  • Addressed:
    • app/controllers/analytics_controller.py & app/services/analytics_service.py: Untyped dict[str, Any] on /dwell/dayparts resolved with Pydantic model DwellDaypartsResponse (docs/standards/code-standards.md §2.3).
    • Mysterious Name: Magic threshold count_in < 30 extracted to DAYPART_LOW_SAMPLE_ENTRIES = 30.
    • Volume tercile collapse: Boundary interpolation with min_step guard added.
    • Hourly timeseries patrol floor: Pre-initialized to 0 instead of patrol guards.
    • ADR 0006: Expanded into comprehensive Architecture Decision Record with YAML frontmatter, Context, Decision Drivers, Considered Options, and Consequences.
    • Frontend UNCALIBRATED state chip: Rendered in app.js and i18n.js when sample count < 14.
  • Acknowledged / Retained by Author:
    • patrol_guard_count: int = 8 retained in function signatures for migration callers.
    • Float literal 0.01265625 (0.1125^2) hardcoded across 5 files deferred as general cleanup.
    • Feature envy in analytics_service.get_dwell_dayparts_async deferred.

Items Missed by Previous Review

  • Closed-Hours Dwell Collapse in Live Overview: In OccupancyManager.get_live_occupancy_async (occupancy_service.py:1680), Little's Law dwell is computed strictly inside if sched_info["is_open"]. When the facility closes at 22:00, is_open becomes false, zeroing avg_occupancy and wiping dwell to 0.0 for the rest of the night (0 min · 10:00–22:00) instead of preserving the completed open-window dwell.
  • Riemann / Ingress Boundary Mismatch: In get_dwell_dayparts_async (analytics_service.py:88), count_in queries [start, end - 0.001], whereas get_cycle_average_occupancy_async queries [start, end]. Boundary events are integrated into occupancy for that daypart but excluded from its ingress denominator.

Standards

Documented Standard Violations (Hard)

  • None. The untyped dict violation on /dwell/dayparts was resolved.

Baseline Smells (Judgement Calls)

  • Speculative Generality (Dead Parameters) (app/schemas/occupancy_models.py:112):
    patrol_guard_count is unused in calculate_proportional_occupancy and calculate_confidence_interval following the zero-floor refactor, yet remains in public signatures.
  • Duplicated Code & Shotgun Surgery (database.py:220, occupancy_repository.py:270, occupancy_models.py:223, 237, occupancy_service.py:868):
    The float literal 0.01265625 (0.1125^2, guardrail variance) is hardcoded in 5 separate locations across schemas, repository defaults, and services instead of referencing a single domain constant.
  • Feature Envy & Duplicated Code (app/services/analytics_service.py:35-128):
    get_dwell_dayparts_async reaches directly into occ_repo and occ_mgr to slice bounds, fetch events, and re-implement Little's Law dwell math (W = \bar{L}/\lambda) already encapsulated in occupancy_service.py.

Spec

Missing or Partial Requirements

  • Live Open-Window Dwell Availability:
    • Quote: Issue #32, line 39: "Compute Ō and W_mean over the retail open window (open_time → close_time...)".
    • Finding: Dwell collapses to 0.0 once is_open becomes false after closing, rather than reporting the completed open-window value until the next day's opening.
  • Dead Parameters in Signatures:
    • Quote: PR #70 Description: "patrol_guard_count used only for nocturnal calibration convergence".
    • Finding: Parameter remains as dead argument across proportional occupancy math.

Scope Creep (Unasked Behaviour)

  • occupancy_definition_cutover_at DB Column & API Fields: Added database column and schema fields across OccupancyCalibrationInfo and dwell responses.
  • Auxiliary Dwell Window Metadata Fields: Added 4 dwell metadata fields (dwell_window_start_epoch, dwell_window_end_epoch, dwell_window_label, dwell_kpi_label) to OccupancyLiveResponse.

Incorrect Implementations

  • Closed-Hour Dwell Erasure in Live Overview:
    • Quote: Issue #32, line 39: "Compute Ō and W_mean over the retail open window".
    • Finding: Gating live dwell calculation on sched_info["is_open"] wipes the metric during evening and nocturnal review.
  • Riemann Integration Boundary Mismatch:
    • Quote: Issue #32, line 39: "Ō and W_mean over the retail open window".
    • Finding: Daypart ingress query uses end - 0.001 while Riemann average uses end, creating off-by-boundary discrepancies between ingress denominator and integrated headcount.

Summary: 3 standards findings (worst: duplicated float literal 0.01265625 and dead patrol_guard_count parameter across multiple layers); 6 spec findings (worst: live dwell collapsing to 0.0 after closing hours when is_open becomes false instead of displaying the completed open-window dwell).

### Review Follow-up & Verification of Previous Findings #### Previous Review Status - **Addressed**: - `app/controllers/analytics_controller.py` & `app/services/analytics_service.py`: Untyped `dict[str, Any]` on `/dwell/dayparts` resolved with Pydantic model `DwellDaypartsResponse` (`docs/standards/code-standards.md §2.3`). - Mysterious Name: Magic threshold `count_in < 30` extracted to `DAYPART_LOW_SAMPLE_ENTRIES = 30`. - Volume tercile collapse: Boundary interpolation with `min_step` guard added. - Hourly timeseries patrol floor: Pre-initialized to 0 instead of patrol guards. - ADR 0006: Expanded into comprehensive Architecture Decision Record with YAML frontmatter, Context, Decision Drivers, Considered Options, and Consequences. - Frontend `UNCALIBRATED` state chip: Rendered in `app.js` and `i18n.js` when sample count $< 14$. - **Acknowledged / Retained by Author**: - `patrol_guard_count: int = 8` retained in function signatures for migration callers. - Float literal `0.01265625` ($0.1125^2$) hardcoded across 5 files deferred as general cleanup. - Feature envy in `analytics_service.get_dwell_dayparts_async` deferred. #### Items Missed by Previous Review - **Closed-Hours Dwell Collapse in Live Overview**: In `OccupancyManager.get_live_occupancy_async` (`occupancy_service.py:1680`), Little's Law dwell is computed strictly inside `if sched_info["is_open"]`. When the facility closes at 22:00, `is_open` becomes false, zeroing `avg_occupancy` and wiping dwell to `0.0` for the rest of the night (`0 min · 10:00–22:00`) instead of preserving the completed open-window dwell. - **Riemann / Ingress Boundary Mismatch**: In `get_dwell_dayparts_async` (`analytics_service.py:88`), `count_in` queries `[start, end - 0.001]`, whereas `get_cycle_average_occupancy_async` queries `[start, end]`. Boundary events are integrated into occupancy for that daypart but excluded from its ingress denominator. --- ## Standards ### Documented Standard Violations (Hard) - None. The untyped dict violation on `/dwell/dayparts` was resolved. ### Baseline Smells (Judgement Calls) - **Speculative Generality (Dead Parameters)** (`app/schemas/occupancy_models.py:112`): `patrol_guard_count` is unused in `calculate_proportional_occupancy` and `calculate_confidence_interval` following the zero-floor refactor, yet remains in public signatures. - **Duplicated Code & Shotgun Surgery** (`database.py:220`, `occupancy_repository.py:270`, `occupancy_models.py:223, 237`, `occupancy_service.py:868`): The float literal `0.01265625` ($0.1125^2$, guardrail variance) is hardcoded in 5 separate locations across schemas, repository defaults, and services instead of referencing a single domain constant. - **Feature Envy & Duplicated Code** (`app/services/analytics_service.py:35-128`): `get_dwell_dayparts_async` reaches directly into `occ_repo` and `occ_mgr` to slice bounds, fetch events, and re-implement Little's Law dwell math ($W = \bar{L}/\lambda$) already encapsulated in `occupancy_service.py`. --- ## Spec ### Missing or Partial Requirements - **Live Open-Window Dwell Availability**: - Quote: Issue #32, line 39: *"Compute Ō and W_mean over the retail open window (open_time → close_time...)"*. - Finding: Dwell collapses to 0.0 once `is_open` becomes false after closing, rather than reporting the completed open-window value until the next day's opening. - **Dead Parameters in Signatures**: - Quote: PR #70 Description: *"patrol_guard_count used only for nocturnal calibration convergence"*. - Finding: Parameter remains as dead argument across proportional occupancy math. ### Scope Creep (Unasked Behaviour) - **`occupancy_definition_cutover_at` DB Column & API Fields**: Added database column and schema fields across `OccupancyCalibrationInfo` and dwell responses. - **Auxiliary Dwell Window Metadata Fields**: Added 4 dwell metadata fields (`dwell_window_start_epoch`, `dwell_window_end_epoch`, `dwell_window_label`, `dwell_kpi_label`) to `OccupancyLiveResponse`. ### Incorrect Implementations - **Closed-Hour Dwell Erasure in Live Overview**: - Quote: Issue #32, line 39: *"Compute Ō and W_mean over the retail open window"*. - Finding: Gating live dwell calculation on `sched_info["is_open"]` wipes the metric during evening and nocturnal review. - **Riemann Integration Boundary Mismatch**: - Quote: Issue #32, line 39: *"Ō and W_mean over the retail open window"*. - Finding: Daypart ingress query uses `end - 0.001` while Riemann average uses `end`, creating off-by-boundary discrepancies between ingress denominator and integrated headcount. --- **Summary**: 3 standards findings (worst: duplicated float literal `0.01265625` and dead `patrol_guard_count` parameter across multiple layers); 6 spec findings (worst: live dwell collapsing to 0.0 after closing hours when `is_open` becomes false instead of displaying the completed open-window dwell).
chore(wip): open draft for passenger flow ingestion honesty (#26, #31, #30)
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m15s
Details
0b6cd98cee 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
chore(wip): open draft for business cycles in facility time (#27)
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m16s
Details
054612b972 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(occupancy): surface ingestion stalls in cycle integrity
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m14s
Details
7cfdcbcf60
fix(occupancy): resolve business cycles in facility time
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m14s
Details
54bc51edef
test: isolate background polling from API database tests
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m12s
Details
6f48205a16
test: isolate background polling from API database tests
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m13s
Details
1463e8c7f5
fix(occupancy): address PR #68 review round 2
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m13s
Details
b2b1d2eefc 
- Spread reconstructed gaps uniformly (#31). _spread_gap_delta cut gaps
  only at hour edges, so an outage inside one clock hour (the 20-minute
  case from #31) still became a single spike at its midpoint. It now
  slices on a 5-minute grid that divides the hour, allocates shares by
  length with largest-remainder rounding (shares sum exactly), and
  never crosses an hourly bucket.
- Type the persisted counter readings. get_passenger_flow_readings_async
  returned untyped dicts and readings travelled as anonymous 7-tuples
  unpacked by index; both now use a PassengerFlowReading model
  (code-standards 2.3).
- Name the reconciliation tuning values (gap threshold, spread step,
  drift cadence, cap floor and multiplier, cold-start limit, stall
  polls) instead of inline literals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	app/db/occupancy_repository.py
#	app/schemas/occupancy_models.py
#	app/services/occupancy_service.py
#	tests/test_counting_integrity.py
fix(occupancy): address PR #69 review round 2
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m12s
Details
30cb43bd57 
- Quiet-window auto-calibration no longer runs before the reset it
  depends on. With the production window 03:30-04:30 around a 04:00
  reset, a run at 03:30 evaluated a cycle still in progress and set the
  per-cycle guard, so the proper post-reset run never happened. The
  daemon now waits until the calibrated cycle has ended when its window
  straddles the reset.
- Route the quiet-window checks through the facility-time seam (#27).
  The daemon and the analytics calibration status both did their own
  hour*60+minute arithmetic; they now use facility_window_containing()
  and seconds_until_window_start(), which handle midnight wrap.
- Tests pin the server clock to UTC with the facility at UTC-4: no run
  at 03:35, a run at 04:15 facility time, and no run at 04:15 server
  time (00:15 facility time).
- Cycle bounds are a CycleBounds named tuple (still unpackable) with a
  next_start property and a named CYCLE_END_EPSILON instead of bare
  +/-0.001 at call sites.
- Rename midnight_epoch to cycle_start_epoch: it holds the cycle reset,
  not midnight.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	CONTEXT.md
#	app/controllers/analytics_controller.py
#	app/db/occupancy_repository.py
#	app/facility_time.py
#	app/schemas/occupancy_models.py
#	app/services/analytics_service.py
#	app/services/occupancy_service.py
#	tests/test_counting_integrity.py
#	tests/test_facility_time.py
fix(occupancy): resolve opening hours for the business day, not the calendar day
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m13s
Details
04dddc099f 
Between midnight and the reset (00:00-04:00 with a 04:00 reset) an
instant still belongs to the previous day's business cycle, but
get_active_schedule_info_async looked up the calendar day's schedule.
From midnight on it returned tomorrow's opening hours: live open-window
dwell read 0 for those hours (found while checking PR #70's review),
and a venue open past midnight was reported closed. The holiday lookup,
weekday and open/close epochs now use the date of the business cycle
that contains the instant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(calibration): address PR #70 review round 2
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m16s
Details
3c6daebfad 
- Live dwell after closing: the review reported it dropping to 0 once
  the venue closes. The evening case already held (the schedule flag is
  per day), but between midnight and the reset the schedule resolved to
  the next calendar day, so dwell read 0 until 04:00. Fixed in #69
  (opening hours resolved for the business day) and merged here; a test
  pins that dwell after midnight equals dwell after closing.
- Remove the dead patrol_guard_count parameter from
  calculate_proportional_occupancy, calculate_confidence_interval,
  calculate_occupancy and the repository peak/average queries, along
  with the unused baseline_offset those queries only used to compute it.
  Since #32 the guard count plays no part in published occupancy.
- One DEFAULT_MULTIPLIER_VARIANCE constant (0.1125^2, the guardrail-
  derived default) replaces 18 copies of the literal across schemas,
  repository defaults, the database seed and services; the guardrail
  bounds are named OPERATIONAL_GUARDRAIL_MIN/MAX where the clamp uses
  them.
- Dayparts read ingress and average occupancy over the same half-open
  slice [start, end).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gabogg commented 2026-09-24 16:47:15 +00:00
Author
Owner
Copy link

Review round 2 addressed — 3c6daeb (plus #68's and #69's fixes merged in)

Spec

  • Live dwell erased after closing: the evening case already held, since is_open is a per-day flag, not "open now". The real defect was after midnight: the schedule resolved to the next calendar day, so dwell read 0 from 00:00 to the 04:00 reset. Fixed in #69 (04dddc0) and merged here. Test test_live_dwell_keeps_completed_open_window_after_closing_and_midnight: 47.6 min after closing and after midnight; a mutation back to calendar-day lookup gives 0.0.
  • Daypart ingress/occupancy boundary: aligned. Both now read the same half-open slice [start, end). The old mismatch was measure-zero for the time integral, but the windows now match exactly.
  • Dead patrol_guard_count: removed from calculate_proportional_occupancy, calculate_confidence_interval, calculate_occupancy and the repository peak/average queries, along with the baseline_offset those queries only used to compute it. Callers and tests updated.

Standards

  • 0.01265625 duplicated: fixed. It was 18 copies (not 5) across schemas, repository defaults, the DB seed and services, now one DEFAULT_MULTIPLIER_VARIANCE with its derivation documented. The guardrail clamp uses OPERATIONAL_GUARDRAIL_MIN/MAX.
  • Left for round 3: feature envy in analytics_service.get_dwell_dayparts_async (moving the dwell math into the occupancy service is a refactor across both services).

Scope-creep findings (occupancy_definition_cutover_at, dwell window metadata fields): recorded as implementation decisions in the description. They answer this PR's open design point 3 (historical discontinuity) and #32's "the KPI self-labels its window".

Verification: pytest 262 passed / 1 skipped, ruff clean, pre-commit passed.

Stacking: #68 → #69 → #70 → #71. Each branch now merges the one below it, so fixes propagate by merge instead of by cherry-pick. Merge in that order.

## Review round 2 addressed — `3c6daeb` (plus #68's and #69's fixes merged in) **Spec** - **Live dwell erased after closing:** the evening case already held, since `is_open` is a per-day flag, not "open now". The real defect was **after midnight**: the schedule resolved to the next calendar day, so dwell read 0 from 00:00 to the 04:00 reset. Fixed in #69 (`04dddc0`) and merged here. Test `test_live_dwell_keeps_completed_open_window_after_closing_and_midnight`: 47.6 min after closing and after midnight; a mutation back to calendar-day lookup gives 0.0. - **Daypart ingress/occupancy boundary: aligned.** Both now read the same half-open slice `[start, end)`. The old mismatch was measure-zero for the time integral, but the windows now match exactly. - **Dead `patrol_guard_count`: removed** from `calculate_proportional_occupancy`, `calculate_confidence_interval`, `calculate_occupancy` and the repository peak/average queries, along with the `baseline_offset` those queries only used to compute it. Callers and tests updated. **Standards** - **`0.01265625` duplicated: fixed.** It was 18 copies (not 5) across schemas, repository defaults, the DB seed and services, now one `DEFAULT_MULTIPLIER_VARIANCE` with its derivation documented. The guardrail clamp uses `OPERATIONAL_GUARDRAIL_MIN/MAX`. - **Left for round 3:** feature envy in `analytics_service.get_dwell_dayparts_async` (moving the dwell math into the occupancy service is a refactor across both services). **Scope-creep findings** (`occupancy_definition_cutover_at`, dwell window metadata fields): recorded as implementation decisions in the description. They answer this PR's open design point 3 (historical discontinuity) and #32's "the KPI self-labels its window". **Verification:** pytest **262 passed / 1 skipped**, ruff clean, pre-commit passed. **Stacking:** #68 → #69 → #70 → #71. Each branch now merges the one below it, so fixes propagate by merge instead of by cherry-pick. Merge in that order.
gabogg commented 2026-09-24 17:09:38 +00:00
Author
Owner
Copy link

Code review — round 3 (pre-merge)

Reviewed this PR's own increment in the stack (#68 → #69 → #70 → #71) with two independent passes: Standards (docs/standards/code-standards.md, AGENTS.md, CONTEXT-FORMAT.md / ADR-FORMAT.md, a code-smell baseline) and Spec (the PR description and its implementation decisions, all previous rounds, and the originating issues). The Spec pass re-ran the suites in a throwaway worktree.

Policy for this round: P1 is fixed before merge; P2 is fixed or explicitly accepted; P3 goes to follow-up issues (inline production values → #74, language-specific text → #73).

Standards

Round-2 fixes hold (no dangling patrol_guard_count / baseline_offset; one DEFAULT_MULTIPLIER_VARIANCE). No hard violations. ADR 0006 and the four new glossary terms follow the formats.

  • P3, one-line fixes: 0.90 / 1.35 copied again in initial_exit_multiplier's Field(...) (occupancy_models.py:234); app.js:3221 re-computes count < 14 instead of reading sample_maturity.is_uncalibrated, which leaves the MODERATE / INITIAL branches unreachable.
  • P3: the threshold 14 in 3 places (→ #74); the Spanish-only fallback 'SIN CALIBRAR' and the server-built dwell_kpi_label (→ #73); an f-string cache key.

Spec

pytest 263, node 67. Round-2 fixes hold.

  • P2, reproduced: the upgrade seeds the wrong k. database.py:201-211 copies the learned active_exit_multiplier into initial_exit_multiplier, so the EWMA is re-seeded with its own output. master's implicit seed was 1.1162.
  • P2, reproduced: the live phased model still floors occupancy at 180 (n_staff_target; k = 1.35, in = out = 1000 gives 180), while ADR 0006 claims a zero floor. The behaviour predates this PR.
  • P3: /dwell/dayparts defaults to the calendar day after midnight; N = 2–13 can still draw a narrow band while UNCALIBRATED; occupancy_definition_cutover_at is questionable, because dwell is recomputed rather than stored; operators get no in-app explanation of the night occupancy dropping from 12 to 0 on deploy day; there is no UI for initial_exit_multiplier.

Standards: 6 findings, worst P3. Spec: 7 findings, worst P2 (wrong seed on upgrade).

Resolution: the seed P2 and the two one-line P3s are being fixed in this PR. The 180-floor P2 is accepted: ADR 0006 is corrected now, and the model change goes to the follow-up issue.

## Code review — round 3 (pre-merge) Reviewed this PR's **own increment** in the stack (#68 → #69 → #70 → #71) with two independent passes: **Standards** (`docs/standards/code-standards.md`, `AGENTS.md`, `CONTEXT-FORMAT.md` / `ADR-FORMAT.md`, a code-smell baseline) and **Spec** (the PR description and its implementation decisions, all previous rounds, and the originating issues). The Spec pass re-ran the suites in a throwaway worktree. Policy for this round: **P1** is fixed before merge; **P2** is fixed or explicitly accepted; **P3** goes to follow-up issues (inline production values → #74, language-specific text → #73). ## Standards Round-2 fixes hold (no dangling `patrol_guard_count` / `baseline_offset`; one `DEFAULT_MULTIPLIER_VARIANCE`). No hard violations. ADR 0006 and the four new glossary terms follow the formats. - **P3, one-line fixes:** `0.90` / `1.35` copied again in `initial_exit_multiplier`'s `Field(...)` (`occupancy_models.py:234`); `app.js:3221` re-computes `count < 14` instead of reading `sample_maturity.is_uncalibrated`, which leaves the `MODERATE` / `INITIAL` branches unreachable. - **P3:** the threshold `14` in 3 places (→ #74); the Spanish-only fallback `'SIN CALIBRAR'` and the server-built `dwell_kpi_label` (→ #73); an f-string cache key. ## Spec pytest 263, node 67. Round-2 fixes hold. - **P2, reproduced: the upgrade seeds the wrong `k`.** `database.py:201-211` copies the learned `active_exit_multiplier` into `initial_exit_multiplier`, so the EWMA is re-seeded with its own output. `master`'s implicit seed was `1.1162`. - **P2, reproduced: the live phased model still floors occupancy at 180** (`n_staff_target`; `k = 1.35`, in = out = 1000 gives 180), while ADR 0006 claims a zero floor. The behaviour predates this PR. - **P3:** `/dwell/dayparts` defaults to the calendar day after midnight; N = 2–13 can still draw a narrow band while `UNCALIBRATED`; `occupancy_definition_cutover_at` is questionable, because dwell is recomputed rather than stored; **operators get no in-app explanation** of the night occupancy dropping from 12 to 0 on deploy day; there is no UI for `initial_exit_multiplier`. --- **Standards: 6 findings, worst P3. Spec: 7 findings, worst P2** (wrong seed on upgrade). **Resolution:** the seed P2 and the two one-line P3s are being fixed in this PR. The 180-floor P2 is **accepted**: ADR 0006 is corrected now, and the model change goes to the follow-up issue.
fix(occupancy): address PR #68 review round 3
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m20s
Details
2e1641bffc 
- P1: a reconstructed gap crossing the cycle rollover was published twice.
  After a rollover the expected cycle total included the slices stamped
  before the reset, so the next drift reconcile republished them at
  "now" (untagged) just after 04:00, the phantom spike #26 targets. The
  expected total now counts only the slices that land in the current
  cycle.
- P2: drift above the cap is no longer carried forward in cap-sized
  pieces. As #26 specifies, it is recorded as a reset anomaly and
  re-seeded, never published; drift within the cap is published once.
- P2: one gap anywhere no longer disables R2 for the whole cycle. The
  hourly trust input excludes reconstructed slices, so a gap cannot fake
  a burst, while the share is still taken over all passages, so it
  cannot hide a real one either.
- The hour-edge spread test asserted a condition that is always true; it
  now pins the exact slices.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs(occupancy): document the quiet-window placement constraint (PR #69 round 3)
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m20s
Details
884aed8e4b 
Round 3 found that calibration_cycle_bounds picks the cycle by calendar
day, so a quiet window far from the reset calibrates a stale cycle and a
window wrapping midnight reads the cycle in progress. Production's
03:30-04:30 window around a 04:00 reset is unaffected and the behaviour
predates this PR; the maintainer accepted it for follow-up. Record the
constraint where it is configured and where it is implemented.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(calibration): address PR #70 review round 3
All checks were successful
CI / lint-and-test (pull_request) Successful in 1m20s
Details
5841296daf 
- P2: upgrading an existing database copied the learned
  active_exit_multiplier into the new initial_exit_multiplier seed, so
  the EWMA was re-seeded with its own output. Existing installations now
  get LEGACY_IMPLICIT_SEED_MULTIPLIER (1.1162), the constant every
  function defaulted to before the seed was configurable (#36); fresh
  databases keep 1.0. Test upgrades a pre-seed database twice.
- P2 (accepted): ADR 0006 claimed the published result has a zero floor.
  The proportional model and its aggregates do, but the live two-phase
  presentation still adds a staff baseline during opening hours; the ADR
  now says so and points to follow-up.
- initial_exit_multiplier bounds use OPERATIONAL_GUARDRAIL_MIN/MAX.
- The dashboard maturity badge branches on the backend's
  sample_maturity.is_uncalibrated instead of re-deriving the threshold,
  which left two branches unreachable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gabogg commented 2026-09-24 22:32:34 +00:00
Author
Owner
Copy link

Review round 3 addressed — 5841296 (plus #68/#69 round-3 fixes merged in)

  • P2, wrong seed on upgrade: fixed. Existing installations get LEGACY_IMPLICIT_SEED_MULTIPLIER (1.1162, the constant every function defaulted to before #36), not their learned multiplier, so the EWMA is no longer re-seeded with its own output. Fresh databases keep 1.0. Test: a pre-seed database upgraded twice keeps active = 1.0842 and seeds 1.1162.
  • P2, the 180 live floor: accepted. ADR 0006 now says the proportional model and its aggregates have a zero floor, while the live two-phase presentation still adds a staff baseline. The decision on the model is tracked in #77.
  • P3 one-liners: fixed. initial_exit_multiplier bounds use OPERATIONAL_GUARDRAIL_MIN/MAX; the maturity badge branches on the backend's is_uncalibrated, which removes the unreachable MODERATE / INITIAL branches.
  • Other P3s → #77; inline values → #74; 'SIN CALIBRAR' and dwell_kpi_label → #73.

Verification: pytest 268 passed / 1 skipped, node 67/67, CI green on 5841296.

## Review round 3 addressed — `5841296` (plus #68/#69 round-3 fixes merged in) - **P2, wrong seed on upgrade: fixed.** Existing installations get `LEGACY_IMPLICIT_SEED_MULTIPLIER` (1.1162, the constant every function defaulted to before #36), not their learned multiplier, so the EWMA is no longer re-seeded with its own output. Fresh databases keep 1.0. Test: a pre-seed database upgraded twice keeps `active = 1.0842` and seeds `1.1162`. - **P2, the 180 live floor: accepted.** ADR 0006 now says the proportional model and its aggregates have a zero floor, while the live two-phase presentation still adds a staff baseline. The decision on the model is tracked in **#77**. - **P3 one-liners: fixed.** `initial_exit_multiplier` bounds use `OPERATIONAL_GUARDRAIL_MIN/MAX`; the maturity badge branches on the backend's `is_uncalibrated`, which removes the unreachable `MODERATE` / `INITIAL` branches. - **Other P3s → #77**; inline values → #74; `'SIN CALIBRAR'` and `dwell_kpi_label` → #73. **Verification:** pytest **268 passed / 1 skipped**, node 67/67, CI green on `5841296`.
gabogg merged commit 9c4f896651 into master 2026-09-24 22:33:07 +00:00
gabogg deleted branch fix/calibration-honesty 2026-09-24 22:33:07 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
blocked
Waits on the issues listed in its "Blocked by:" line before implementation

  
bug
Category: something is broken; behaviour differs from the spec or docs

  
enhancement
Category: new feature or improvement, including refactors, chores and docs

  
high-priority
Address before ordinary-priority work; independent of triage readiness and blockers

  
low-priority
Address after ordinary-priority work; independent of triage readiness and blockers

  
needs-info
Waiting on reporter for more information

  
needs-triage
Maintainer needs to evaluate this issue

  
ready-for-agent
Fully specified, ready for an AFK agent

  
ready-for-human
Requires human implementation

  
referenced
Previously closed issue reopened or referenced by an open issue; visible history marker

  
research
Needs investigation (internet or local resources) before it can be triaged down; used alongside a category and a triage role

  
wontfix
Will not be actioned

No labels
blocked
bug
enhancement
high-priority
low-priority
needs-info
needs-triage
ready-for-agent
ready-for-human
referenced
research
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gabogg/hikcentral!70
No description provided.