gabogg/hikcentral

Watch

Fork 0

refactor(config): replace inline production values with named config keys and constants #74

New issue

Open

opened 2026-09-24 13:13:53 +00:00 by gabogg · 2 comments

gabogg commented

2026-09-24 13:13:53 +00:00

Owner

Requested by the maintainer on 2026-09-24: once every [data-veracity] issue is closed, sweep the codebase programmatically for magic values, i.e. production values written inline where the code should reference a named config key or constant. Agents tend to write the current site's value (1.1162, "04:00") instead of the name of the setting it comes from. Several [data-veracity] issues already move specific values into config (#36: initial_exit_multiplier; #33: trust thresholds), so this sweep runs after them and catches what remains. Needs triage before work starts.

Principle

A value that belongs to the deployment (a site's calibration, schedule, thresholds) lives in occupancy_config / settings and is read by name. A value that is a unit or protocol constant (seconds per hour) gets a named constant, defined once. Literals repeated as fallbacks (cfg.get("x", <production value>)) should take their default from one place.

Evidence from a first scan (`master` at `42891fa`; occurrences in `app/` Python and JS)

Value	Meaning	Occurrences	Files
`1.1162`	this mall's measured exit multiplier `k`	19	database, repository, schemas, analytics, occupancy service, prototype
`0.0002`	default multiplier variance	17	database, repository, schemas, analytics, occupancy service
`"04:00"`	daily reset time fallback	17	repository, analytics, occupancy service, prototype
`"03:30"` / `"04:30"`	calibration window fallbacks	11 / 10	controller, repository, schemas, analytics, occupancy service, prototype
`86400`	seconds per day / cycle length	27	8 files (see #27 on cycle length)
`3600`	seconds per hour	18	10 files incl. JS
`5000` / `40000`	trust footfall floor / "counter flush" ceiling	5 / 2	occupancy service, repository, database
`0.35`, `0.80`, `1.30`	trust rule thresholds	4 / 9 / 5	backend and `static/js/src/ui/calibration_desk.js`, `app.js`
`0.90`, `1.35`	operational calibration guardrail	4 / 3	occupancy service, `telemetry_engine.js`
`900.0`	evaluation offset before cycle end	4	occupancy service

Contradiction already visible: commit 3c0c519 changed the daily_reset_time config default to "00:00" (HikCentral's reported time), but code still falls back to "04:00" in 17 places. Any path that reaches the fallback resets at the wrong hour.

Duplicated across the stack: the frontend hardcodes backend thresholds (calibration_desk.js: 0.35, 0.80, 1.30; telemetry_engine.js: 0.90). They should come from the API (config) so the two can't drift.

Scope (to be settled in triage)

A repeatable scan (script or test) listing numeric/time literals that match known config values, so reintroductions are caught in CI.
Deployment values → read from config by name, with one default definition (e.g. the schema/occupancy_config defaults), never a repeated literal.
Unit constants → named constants (SECONDS_PER_HOUR, …) in one module; cycle length follows #27's decision.
Frontend thresholds → served by the API, not duplicated in JS.

Open questions for triage

Where does the single source of defaults live: occupancy_config column defaults, a Pydantic settings model, or a constants module?
Which values are genuinely per-deployment (config) vs fixed domain rules (constants)?
Does the "04:00" vs "00:00" contradiction need a fix before this sweep, as a data-veracity bug of its own?
Scope of the scan: app/ only, or tests and prototype_* files too (prototype_occupancy_math.py carries many of these)?

Related: #27, #33, #36.

Triage decisions (2026-09-24)

The earlier blockers (#26, #27, #30–#34, #36) are closed; this issue is unblocked.
Single source: deployment defaults in app/config_defaults.py; unit constants (SECONDS_PER_HOUR, …) in app/units.py. DDL, schemas and services all import from these; no value is written twice.
Settings vs fixed rules: site-tunable values are config (seed k, multiplier variance, reset, calibration window, trust thresholds, 5000 / 40000 footfall bounds). The operational guardrail (0.90–1.35), the 900 s evaluation offset and the 14-cycle maturity threshold are named constants; the frontend receives both from the API instead of hardcoding them.
Reset default: "04:00" for fresh deployments (safe for venues open past midnight; matches production). The HikCentral-reported time is a hint shown to the operator, not the default. Resolves #76 item 2.
Scan: app/ Python plus frontend JS, as a CI test. Tests are excluded; prototype_* files are excluded and, if nothing imports them, deleted in a separate chore.

> Requested by the maintainer on 2026-09-24: once every `[data-veracity]` issue is closed, sweep the codebase **programmatically** for **magic values**, i.e. production values written inline where the code should reference a named config key or constant. Agents tend to write the current site's value (`1.1162`, `"04:00"`) instead of the name of the setting it comes from. Several `[data-veracity]` issues already move specific values into config (#36: `initial_exit_multiplier`; #33: trust thresholds), so this sweep runs **after** them and catches what remains. **Needs triage** before work starts. ## Principle A value that belongs to the **deployment** (a site's calibration, schedule, thresholds) lives in `occupancy_config` / settings and is read **by name**. A value that is a **unit or protocol constant** (seconds per hour) gets a named constant, defined once. Literals repeated as fallbacks (`cfg.get("x", <production value>)`) should take their default from **one** place. ## Evidence from a first scan (`master` at `42891fa`; occurrences in `app/` Python and JS) | Value | Meaning | Occurrences | Files | |---|---|---|---| | `1.1162` | this mall's measured exit multiplier `k` | 19 | database, repository, schemas, analytics, occupancy service, prototype | | `0.0002` | default multiplier variance | 17 | database, repository, schemas, analytics, occupancy service | | `"04:00"` | daily reset time fallback | 17 | repository, analytics, occupancy service, prototype | | `"03:30"` / `"04:30"` | calibration window fallbacks | 11 / 10 | controller, repository, schemas, analytics, occupancy service, prototype | | `86400` | seconds per day / cycle length | 27 | 8 files (see #27 on cycle length) | | `3600` | seconds per hour | 18 | 10 files incl. JS | | `5000` / `40000` | trust footfall floor / "counter flush" ceiling | 5 / 2 | occupancy service, repository, database | | `0.35`, `0.80`, `1.30` | trust rule thresholds | 4 / 9 / 5 | backend **and** `static/js/src/ui/calibration_desk.js`, `app.js` | | `0.90`, `1.35` | operational calibration guardrail | 4 / 3 | occupancy service, `telemetry_engine.js` | | `900.0` | evaluation offset before cycle end | 4 | occupancy service | **Contradiction already visible:** commit `3c0c519` changed the `daily_reset_time` config default to `"00:00"` (HikCentral's reported time), but code still falls back to `"04:00"` in 17 places. Any path that reaches the fallback resets at the wrong hour. **Duplicated across the stack:** the frontend hardcodes backend thresholds (`calibration_desk.js`: `0.35`, `0.80`, `1.30`; `telemetry_engine.js`: `0.90`). They should come from the API (config) so the two can't drift. ## Scope (to be settled in triage) 1. A **repeatable scan** (script or test) listing numeric/time literals that match known config values, so reintroductions are caught in CI. 2. Deployment values → read from config by name, with **one** default definition (e.g. the schema/`occupancy_config` defaults), never a repeated literal. 3. Unit constants → named constants (`SECONDS_PER_HOUR`, …) in one module; cycle length follows #27's decision. 4. Frontend thresholds → served by the API, not duplicated in JS. ## Open questions for triage - Where does the single source of defaults live: `occupancy_config` column defaults, a Pydantic settings model, or a constants module? - Which values are genuinely per-deployment (config) vs fixed domain rules (constants)? - Does the `"04:00"` vs `"00:00"` contradiction need a fix before this sweep, as a data-veracity bug of its own? - Scope of the scan: `app/` only, or tests and `prototype_*` files too (`prototype_occupancy_math.py` carries many of these)? Related: #27, #33, #36. ## Triage decisions (2026-09-24) - The earlier blockers (#26, #27, #30–#34, #36) are closed; this issue is unblocked. - **Single source:** deployment defaults in `app/config_defaults.py`; unit constants (`SECONDS_PER_HOUR`, …) in `app/units.py`. DDL, schemas and services all import from these; no value is written twice. - **Settings vs fixed rules:** site-tunable values are config (seed k, multiplier variance, reset, calibration window, trust thresholds, 5000 / 40000 footfall bounds). The operational guardrail (0.90–1.35), the 900 s evaluation offset and the 14-cycle maturity threshold are named constants; the frontend receives both from the API instead of hardcoding them. - **Reset default:** `"04:00"` for fresh deployments (safe for venues open past midnight; matches production). The HikCentral-reported time is a hint shown to the operator, not the default. Resolves #76 item 2. - **Scan:** `app/` Python plus frontend JS, as a CI test. Tests are excluded; `prototype_*` files are excluded and, if nothing imports them, deleted in a separate chore.

gabogg added the

needs-triage

label

2026-09-24 13:13:54 +00:00

gabogg commented

2026-09-24 13:16:50 +00:00

Author

Owner

Triage note (2026-09-24): the "04:00" vs "00:00" reset-time contradiction is latent, not urgent.

Production stores daily_reset_time = 04:00 explicitly (confirmed by the maintainer), so the 17 inline "04:00" fallbacks currently agree with production. A fresh deployment gets the "00:00" column default from the database, so the key is present there too. The literal fallback only takes effect if the config row is missing or incomplete.

Decision: no fix in the open [data-veracity] PRs (#68–#71). They are expected to add more inline values; this sweep runs after them and catches everything. During their reviews, any newly introduced inline production value is flagged as P3 and routed here instead of being fixed in the PR.

**Triage note (2026-09-24): the `"04:00"` vs `"00:00"` reset-time contradiction is latent, not urgent.** Production stores `daily_reset_time = 04:00` explicitly (confirmed by the maintainer), so the 17 inline `"04:00"` fallbacks currently *agree* with production. A fresh deployment gets the `"00:00"` column default from the database, so the key is present there too. The literal fallback only takes effect if the config row is missing or incomplete. Decision: **no fix in the open `[data-veracity]` PRs (#68–#71).** They are expected to add more inline values; this sweep runs after them and catches everything. During their reviews, any *newly introduced* inline production value is flagged as P3 and routed here instead of being fixed in the PR.

gabogg referenced this issue

2026-09-24 16:47:25 +00:00

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

gabogg referenced this issue

2026-09-24 17:09:35 +00:00

fix(occupancy): passenger flow ingestion honesty — counter resets, outage gaps, peak ordering (#26, #31, #30) #68

gabogg referenced this issue

2026-09-24 17:09:37 +00:00

fix(occupancy): business cycles in facility time (#27) #69

gabogg referenced this issue

2026-09-24 17:09:38 +00:00

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

gabogg referenced this issue

2026-09-24 17:09:39 +00:00

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

gabogg referenced this issue

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

refactor(occupancy): PR #68 review follow-ups — sync method size, ingestion flag and anomaly-kind types, cold-start labelling #75

gabogg referenced this issue

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

fix(occupancy): PR #69 review follow-ups — quiet windows away from the reset, cycle-bounds API, CI timezone #76

gabogg referenced this issue

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

fix(occupancy): PR #70 review follow-ups — live staff-baseline floor, operator communication, dwell edge cases #77

gabogg referenced this issue

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

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

gabogg commented

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

Author

Owner

Additions from the round-3 reviews of the [data-veracity] PRs (2026-09-24), per the routing rule in comment 1622:

#68: trailing-rate window now - 900 and / 15.0 inline in get_group_recent_rate_async (not tied to each other); one more cfg.get("daily_reset_time", "04:00") fallback in the stall path; the "%I:%M:%S %p" format string now also in the repository (presentation formatting in the persistence layer).
#69: "04:00" default arguments across app/facility_time.py and three repository call sites; the "AUTOMATIC_NOCTURNAL" string compare in the repository.
#70: the uncalibrated threshold 14 in three places (service maturity check, trusted-history limit, app.js badge text).
#71: the calibration-and-trust threshold inputs in index.html carry value="…" defaults: a fourth copy of the trust defaults (with DDL, repository and schema). The TrustSettings grouping is in the PR #71 follow-up issue; do both together.

**Additions from the round-3 reviews of the `[data-veracity]` PRs (2026-09-24)**, per the routing rule in comment 1622: - **#68:** trailing-rate window `now - 900` and `/ 15.0` inline in `get_group_recent_rate_async` (not tied to each other); one more `cfg.get("daily_reset_time", "04:00")` fallback in the stall path; the `"%I:%M:%S %p"` format string now also in the repository (presentation formatting in the persistence layer). - **#69:** `"04:00"` default arguments across `app/facility_time.py` and three repository call sites; the `"AUTOMATIC_NOCTURNAL"` string compare in the repository. - **#70:** the uncalibrated threshold `14` in three places (service maturity check, trusted-history limit, `app.js` badge text). - **#71:** the calibration-and-trust threshold inputs in `index.html` carry `value="…"` defaults: a fourth copy of the trust defaults (with DDL, repository and schema). The `TrustSettings` grouping is in the PR #71 follow-up issue; do both together.

gabogg referenced this issue

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

fix(occupancy): passenger flow ingestion honesty — counter resets, outage gaps, peak ordering (#26, #31, #30) #68

gabogg referenced this issue

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

fix(occupancy): business cycles in facility time (#27) #69

gabogg referenced this issue

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

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

gabogg referenced this issue

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

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