refactor(health): load generated health registry in fork#11
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR migrates Key changes:
Issue found:
Confidence Score: 3/5Safe to merge after the HEALTH_BOOTSTRAP_ADDITIONS bucket/on-demand mismatch is resolved — one targeted one-line fix required. The overall migration is well-engineered: the generator validates, the check script guards CI, and the algorithm in health.js is untouched. However, there is a concrete behavioral bug: the four keys added via HEALTH_BOOTSTRAP_ADDITIONS are emitted into HEALTH_BOOTSTRAP_KEYS with an on-demand flag that the bootstrap loop never reads. An empty key incorrectly increments critCount instead of warnCount, potentially flipping the overall health status from WARNING to DEGRADED or UNHEALTHY. No existing test catches this gap. The fix is a one-line change (bucket: 'standalone'). registry/datasets.ts — specifically the HEALTH_BOOTSTRAP_ADDITIONS loop in buildDatasets() and the HEALTH_ON_DEMAND_KEYS.push() call at line 527. Important Files Changed
Sequence DiagramsequenceDiagram
participant DTS as registry/datasets.ts
participant GEN as generate-dataset-registry.ts
participant HR as api/_generated/health-registry.js
participant HJS as api/health.js
participant Redis as Redis (Upstash)
Note over DTS: HEALTH_BOOTSTRAP_KEYS<br/>HEALTH_BOOTSTRAP_ADDITIONS (onDemand:true)<br/>HEALTH_STANDALONE_KEYS<br/>HEALTH_SEED_META / on-demand / empty-ok / cascade
DTS->>GEN: DATASETS[]
GEN->>GEN: validateGlobal() — dedup ids, keys, aliases
GEN->>HR: emit HEALTH_BOOTSTRAP_KEYS + HEALTH_STANDALONE_KEYS<br/>HEALTH_SEED_META + HEALTH_ON_DEMAND_KEYS<br/>HEALTH_EMPTY_OK_KEYS + HEALTH_CASCADE_GROUPS
HJS->>HR: import all six exports
HJS->>Redis: STRLEN all data keys + GET all seed-meta keys (pipeline)
Redis-->>HJS: byte lengths + seed-meta payloads
Note over HJS: Bootstrap loop — no on-demand check<br/>ddosAttacks/trafficAnomalies/cryptoSectors/economicStress<br/>in HEALTH_BOOTSTRAP_KEYS AND HEALTH_ON_DEMAND_KEYS<br/>but onDemand flag silently dropped → EMPTY (crit)
HJS->>HJS: Standalone loop — checks ON_DEMAND_KEYS correctly
HJS-->>HJS: overall: HEALTHY / WARNING / DEGRADED / UNHEALTHY
Prompt To Fix All With AIThis is a comment left during a code review.
Path: registry/datasets.ts
Line: 596-597
Comment:
**On-demand flag silently dropped for bootstrap health additions**
`ddosAttacks`, `trafficAnomalies`, `cryptoSectors`, and `economicStress` are registered with `bucket: 'bootstrap'` and `onDemand: true`. The generator correctly emits them in both `HEALTH_BOOTSTRAP_KEYS` and `HEALTH_ON_DEMAND_KEYS`. However, `api/health.js` only consults `ON_DEMAND_KEYS` inside the **standalone** loop (lines 160, 218–221). The **bootstrap** loop (lines 84–154) has no on-demand check:
```js
// health.js bootstrap loop — missing on-demand guard
} else if (!hasData) {
if (EMPTY_DATA_OK_KEYS.has(name)) { ... }
else {
status = 'EMPTY'; // ← should be EMPTY_ON_DEMAND for on-demand keys
critCount++;
}
}
```
This means if any of these four keys is empty (e.g. Cloudflare Radar outage, upstream refresh failure), health.js reports `EMPTY` (critical, increments `critCount`) instead of the intended `EMPTY_ON_DEMAND` (warning). When `critCount` is 1–3 the overall status becomes `DEGRADED`; above 3 it becomes `UNHEALTHY` — rather than the `WARNING` that on-demand semantics are designed to produce.
Two possible fixes:
**Option A — change the bucket to `'standalone'`** (the bootstrap _cache_ bucket and the health _monitoring_ bucket are orthogonal):
```suggestion
register(logicalName, redisKey).health = { bucket: 'standalone', onDemand: true };
```
**Option B — add an on-demand guard to the bootstrap loop in `api/health.js`** (lines 125–128 and 138–141):
```js
} else if (!hasData) {
if (EMPTY_DATA_OK_KEYS.has(name)) { ... }
else if (ON_DEMAND_KEYS.has(name)) {
status = 'EMPTY_ON_DEMAND';
warnCount++;
} else {
status = 'EMPTY';
critCount++;
}
}
```
Option A is the lower-risk change — it requires no modification to `api/health.js`'s runtime logic.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: registry/datasets.ts
Line: 527
Comment:
**Pushed on-demand keys have no runtime effect**
The four keys pushed here — `cryptoSectors`, `ddosAttacks`, `economicStress`, `trafficAnomalies` — are all in `HEALTH_BOOTSTRAP_KEYS` (bucket: `'bootstrap'`). Because `api/health.js` only evaluates `ON_DEMAND_KEYS` inside the standalone loop, listing them here produces no runtime effect. The `push()` call also bypasses TypeScript's readonly inference.
If the intent is truly on-demand monitoring semantics, the bucket for these entries should be `'standalone'` (see the P1 comment above). If critical-when-empty is acceptable for all bootstrap health keys, this `push` call and the `onDemand: true` flag in `HEALTH_BOOTSTRAP_ADDITIONS` should both be removed to avoid misleading future contributors.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "refactor(health): load generated health ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const [logicalName, redisKey] of Object.entries(HEALTH_BOOTSTRAP_ADDITIONS)) { | ||
| register(logicalName, redisKey).health = { bucket: 'bootstrap', onDemand: true }; |
There was a problem hiding this comment.
ddosAttacks, trafficAnomalies, cryptoSectors, and economicStress are registered with bucket: 'bootstrap' and onDemand: true. The generator correctly emits them in both HEALTH_BOOTSTRAP_KEYS and HEALTH_ON_DEMAND_KEYS. However, api/health.js only consults ON_DEMAND_KEYS inside the standalone loop (lines 160, 218–221). The bootstrap loop (lines 84–154) has no on-demand check:
// health.js bootstrap loop — missing on-demand guard
} else if (!hasData) {
if (EMPTY_DATA_OK_KEYS.has(name)) { ... }
else {
status = 'EMPTY'; // ← should be EMPTY_ON_DEMAND for on-demand keys
critCount++;
}
}This means if any of these four keys is empty (e.g. Cloudflare Radar outage, upstream refresh failure), health.js reports EMPTY (critical, increments critCount) instead of the intended EMPTY_ON_DEMAND (warning). When critCount is 1–3 the overall status becomes DEGRADED; above 3 it becomes UNHEALTHY — rather than the WARNING that on-demand semantics are designed to produce.
Two possible fixes:
Option A — change the bucket to 'standalone' (the bootstrap cache bucket and the health monitoring bucket are orthogonal):
| for (const [logicalName, redisKey] of Object.entries(HEALTH_BOOTSTRAP_ADDITIONS)) { | |
| register(logicalName, redisKey).health = { bucket: 'bootstrap', onDemand: true }; | |
| register(logicalName, redisKey).health = { bucket: 'standalone', onDemand: true }; |
Option B — add an on-demand guard to the bootstrap loop in api/health.js (lines 125–128 and 138–141):
} else if (!hasData) {
if (EMPTY_DATA_OK_KEYS.has(name)) { ... }
else if (ON_DEMAND_KEYS.has(name)) {
status = 'EMPTY_ON_DEMAND';
warnCount++;
} else {
status = 'EMPTY';
critCount++;
}
}Option A is the lower-risk change — it requires no modification to api/health.js's runtime logic.
Prompt To Fix With AI
This is a comment left during a code review.
Path: registry/datasets.ts
Line: 596-597
Comment:
**On-demand flag silently dropped for bootstrap health additions**
`ddosAttacks`, `trafficAnomalies`, `cryptoSectors`, and `economicStress` are registered with `bucket: 'bootstrap'` and `onDemand: true`. The generator correctly emits them in both `HEALTH_BOOTSTRAP_KEYS` and `HEALTH_ON_DEMAND_KEYS`. However, `api/health.js` only consults `ON_DEMAND_KEYS` inside the **standalone** loop (lines 160, 218–221). The **bootstrap** loop (lines 84–154) has no on-demand check:
```js
// health.js bootstrap loop — missing on-demand guard
} else if (!hasData) {
if (EMPTY_DATA_OK_KEYS.has(name)) { ... }
else {
status = 'EMPTY'; // ← should be EMPTY_ON_DEMAND for on-demand keys
critCount++;
}
}
```
This means if any of these four keys is empty (e.g. Cloudflare Radar outage, upstream refresh failure), health.js reports `EMPTY` (critical, increments `critCount`) instead of the intended `EMPTY_ON_DEMAND` (warning). When `critCount` is 1–3 the overall status becomes `DEGRADED`; above 3 it becomes `UNHEALTHY` — rather than the `WARNING` that on-demand semantics are designed to produce.
Two possible fixes:
**Option A — change the bucket to `'standalone'`** (the bootstrap _cache_ bucket and the health _monitoring_ bucket are orthogonal):
```suggestion
register(logicalName, redisKey).health = { bucket: 'standalone', onDemand: true };
```
**Option B — add an on-demand guard to the bootstrap loop in `api/health.js`** (lines 125–128 and 138–141):
```js
} else if (!hasData) {
if (EMPTY_DATA_OK_KEYS.has(name)) { ... }
else if (ON_DEMAND_KEYS.has(name)) {
status = 'EMPTY_ON_DEMAND';
warnCount++;
} else {
status = 'EMPTY';
critCount++;
}
}
```
Option A is the lower-risk change — it requires no modification to `api/health.js`'s runtime logic.
How can I resolve this? If you propose a fix, please make it concise.
| 'resilienceRanking', | ||
| ] as string[]; | ||
|
|
||
| HEALTH_ON_DEMAND_KEYS.push('cryptoSectors', 'ddosAttacks', 'economicStress', 'trafficAnomalies'); |
There was a problem hiding this comment.
The four keys pushed here — cryptoSectors, ddosAttacks, economicStress, trafficAnomalies — are all in HEALTH_BOOTSTRAP_KEYS (bucket: 'bootstrap'). Because api/health.js only evaluates ON_DEMAND_KEYS inside the standalone loop, listing them here produces no runtime effect. The push() call also bypasses TypeScript's readonly inference.
If the intent is truly on-demand monitoring semantics, the bucket for these entries should be 'standalone' (see the P1 comment above). If critical-when-empty is acceptable for all bootstrap health keys, this push call and the onDemand: true flag in HEALTH_BOOTSTRAP_ADDITIONS should both be removed to avoid misleading future contributors.
Prompt To Fix With AI
This is a comment left during a code review.
Path: registry/datasets.ts
Line: 527
Comment:
**Pushed on-demand keys have no runtime effect**
The four keys pushed here — `cryptoSectors`, `ddosAttacks`, `economicStress`, `trafficAnomalies` — are all in `HEALTH_BOOTSTRAP_KEYS` (bucket: `'bootstrap'`). Because `api/health.js` only evaluates `ON_DEMAND_KEYS` inside the standalone loop, listing them here produces no runtime effect. The `push()` call also bypasses TypeScript's readonly inference.
If the intent is truly on-demand monitoring semantics, the bucket for these entries should be `'standalone'` (see the P1 comment above). If critical-when-empty is acceptable for all bootstrap health keys, this `push` call and the `onDemand: true` flag in `HEALTH_BOOTSTRAP_ADDITIONS` should both be removed to avoid misleading future contributors.
How can I resolve this? If you propose a fix, please make it concise.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b801fcb45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const [logicalName, redisKey] of Object.entries(HEALTH_BOOTSTRAP_ADDITIONS)) { | ||
| register(logicalName, redisKey).health = { bucket: 'bootstrap', onDemand: true }; | ||
| } |
There was a problem hiding this comment.
These datasets are marked onDemand but still assigned bucket: 'bootstrap', and api/health.js only applies on-demand downgrade logic in the standalone loop. That means a missing value for any of these newly added keys is treated as EMPTY (critical) instead of EMPTY_ON_DEMAND (warning), so cold/evicted caches can incorrectly push overall health to DEGRADED/UNHEALTHY.
Useful? React with 👍 / 👎.
Expand the dataset contract source to cover the existing health semantics, emit an edge-safe health registry, and switch api/health.js to imports without changing the status algorithm. Validation: npm run registry:check; node --test tests/bootstrap.test.mjs tests/market-breadth.test.mjs tests/customs-revenue.test.mjs tests/resilience-static-seed.test.mjs; node --test tests/edge-functions.test.mjs; npm run typecheck; npm run typecheck:api
Route HEALTH_BOOTSTRAP_ADDITIONS through the standalone bucket so api/health.js downgrades cold or evicted data to EMPTY_ON_DEMAND instead of EMPTY. Add a regression test covering the generated bucket assignment for the on-demand additions.
lspassos1
force-pushed
the
refactor/health-generated-registry
branch
from
2b801fc to
141a159
Compare
|
PR author is not in the allowed authors list. |
1 similar comment
|
PR author is not in the allowed authors list. |
1 participant
Summary
This stacks the health migration on top of the bootstrap registry foundation. The existing
api/health.jsstatus algorithm stays intact, but its dataset registrations now come from generated artifacts derived fromregistry/datasets.ts.Root cause
api/health.jscarried a second hand-maintained dataset contract alongside bootstrap. That split forced health-only metadata, on-demand flags, empty-ok semantics, and cascade groups to drift separately from the bootstrap registry source.Changes
registry/datasets.tsto cover the existing health bootstrap and standalone registrationsapi/_generated/health-registry.jsfrom the registry generatorregistry:checkfocused on generated artifacts onlyapi/health.jswith generated importsValidation
npm run registry:checknode --test tests/bootstrap.test.mjs tests/market-breadth.test.mjs tests/customs-revenue.test.mjs tests/resilience-static-seed.test.mjsnode --test tests/edge-functions.test.mjsnpm run typechecknpm run typecheck:apiRisk
Low to moderate. The runtime algorithm is intentionally unchanged, but the generated registry now owns health bootstrap keys, standalone keys, seed metadata, on-demand keys, empty-ok keys, and cascade groups.
Refs #5
Depends on #10