fix(kpi): reject malformed JSON-looking compute evidence - #481
Conversation
|
Warning Review limit reached
Next review available in: 2 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (/^\s*[\[{]/.test(line)) { | ||
| console.error(`Malformed JSON in KPI log line ${index + 1}.`); | ||
| process.exit(1); |
There was a problem hiding this comment.
🟡 Malformed-JSON diagnostic reports wrong line number
filter(Boolean) drops blank lines before the loop, so the iteration index no longer matches the file line. When a blank line precedes a malformed record, Malformed JSON in KPI log line ${index + 1} names the wrong line and sends an operator to the wrong place.
Prompt for agents
The malformed-JSON diagnostic in scripts/compute-kpi.mjs reports a line number derived from the index of the post-filter array. Line 20 builds `lines` as `text.split("\n").filter(Boolean)`, which removes empty lines, so `index` from `lines.entries()` (line 25) no longer corresponds to the actual line number in the source file. When blank lines appear before a malformed record, the number in `Malformed JSON in KPI log line ${index + 1}` (line 31) is off. Fix by tracking the true line number, e.g. iterate over the unfiltered `text.split("\n")` result with its index and skip empty lines inside the loop, so the reported line number matches the file.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (/^\s*[\[{]/.test(line)) { | ||
| console.error(`Malformed JSON in KPI log line ${index + 1}.`); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
🔍 Bracket-prefixed Wrangler diagnostics can fail closed
The new guard treats any non-parseable line starting with { or [ as fatal. Wrangler tail can emit bracket-prefixed diagnostic lines like [wrangler:info] ...; such a line would now exit 1, contradicting the intent to tolerate diagnostic noise. Depends on the actual capture format — confirm the tail output never produces bracket-prefixed non-JSON lines.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Purpose
Test-first repair of direct KPI evidence integrity.
scripts/compute-kpi.mjsintentionally tolerates plain Wrangler diagnostic noise, but its prior JSON parse catch also silently dropped malformed JSON-looking NDJSON. A truncated/exchangefailure record could therefore disappear from request/failure/latency computation instead of failing closed.RED → GREEN
52e786e48cced049f8daf0505b87241d832c4fc7896b145574f05b0e3c5061fab1af032424b8959432608870644, job97118404115; the realistic malformed-JSON regression failed becausecompute-kpiexited 0 instead of 1d2c6e659ca94c69b6a0ba558b1aee373ba51495a{or[after whitespace) but cannot parse, fail closed with a line-specific diagnostic; ordinary Wrangler diagnostic noise remains toleratedscripts/compute-kpi.mjsplustest/compute-kpi-input-integrity.test.tsonlyExact-head verification
On unchanged
d2c6e659ca94c69b6a0ba558b1aee373ba51495a:32609156503: terminal success32609156610: terminal success32609156526: terminal success52e786e48cced049f8daf0505b87241d832c4fc7Authority boundary
This hardens direct retained KPI computation only. It does not create a production 30-day KPI window, provenance, release, deployment, revenue, legal/IP or acquisition-ready evidence. No central
.githubor other dedicated-writer source is modified.