Fix critique routing snapshot metrics

[pbakaus]

Closes #561.

What changed

• Read the documented critique snapshot keys (total_score, p0_count, and p1_count) in context-signals.mjs.
• Preserve compatibility with legacy score, p0, and p1 snapshots.
• Report absent metrics as null instead of silently turning them into plausible zeroes.

Reproduction

Before the fix, a stored 24/40 critique with two P0 and five P1 findings produced score: 0, p0: 0, p1: 0. The same reproduction now produces score: 24, p0: 2, p1: 5.

Validation

• node --test tests/context-signals.test.mjs — 37/37 passed
• bun run build — passed
• bun run test — passed (core, detector/browser, live, framework, and plugin E2E; two documented live skips)
• git diff --check — passed
• bun run test:skill-behavior — executed as required; current-model stochastic scenarios reported unrelated failures, and every failing trace omitted context-signals.mjs while the suite contains no critique/context-signals fixture

Generated provider and root harness output was intentionally omitted under the source-first contribution policy.

Prepared with AI assistance under maintainer @pbakaus's standing automation authorization.

---

Note

Low Risk
Small, test-covered change to signal parsing for critique snapshots; no auth, persistence, or routing logic beyond metadata field names.

Overview
Fixes critique routing for bare /impeccable by aligning latestCritique in context-signals.mjs with the snapshot format documented for critique storage (total_score, p0_count, p1_count), while still accepting legacy score / p0 / p1 front matter.

Numeric parsing now treats missing, blank, or non-numeric metadata as null instead of coercing to 0, so agents no longer see plausible zero scores when a snapshot actually has values like 24/40 with 2 P0 and 5 P1 findings.

Adds unit tests for the documented keys, absent metrics, and empty/invalid values.

^{Reviewed by Cursor Bugbot for commit 357f358. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Copilot]

Pull request overview

Fixes critique routing snapshot metrics by aligning context-signals.mjs with the documented critique snapshot frontmatter keys, while keeping backward compatibility and avoiding silent “0” fallbacks when metrics are absent.

Changes:

• Read total_score, p0_count, and p1_count (with fallback to legacy score, p0, p1) when summarizing the latest critique snapshot.
• Treat missing critique metrics as null (instead of converting null/missing to 0 via numeric coercion).
• Add regression tests covering documented keys and missing-metrics behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
skill/scripts/context-signals.mjs	Fixes critique snapshot metric parsing (documented keys + legacy fallback + null for missing).
tests/context-signals.test.mjs	Adds coverage for documented critique keys and null behavior when metrics are absent.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[greptile-apps]

Greptile Summary

Critique snapshot routing now prefers the documented metric names while retaining legacy aliases. Empty, missing, malformed, and non-finite metric values resolve to null instead of being interpreted as usable scores.

Confidence Score: 5/5

No blocking failure remains.

Focused checks exercised documented-key precedence, legacy compatibility, missing and malformed values, and non-finite metrics. The context-signals test suite completed successfully.

T-Rex Logs

What T-Rex did

• The standalone probe was executed against the parent worktree and PR head using identical generated snapshot fixtures, revealing that the PR head passed all six contract cases while the parent failed on the empty-score case (score: 0).
• The Node tests for context-signals.test.mjs ran on Node 24.19.0 and all 38 tests passed with exit code 0.
• The standalone probe was re-run against the parent worktree and PR head for the changed module, and the results again showed the parent failing on the empty-score case while the PR head passed all six contract cases.
• The focused test file for the changed module was executed directly under Node 24.19.0, and the run exited with code 0.

_{Ran code and verified through T-Rex}

_{Reviews (2): Last reviewed commit: "Reject empty critique metrics" | Re-trigger Greptile}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

skill/scripts/context-signals.mjs:48

• num() still coerces empty-string frontmatter values (e.g. total_score: with no value) to 0 because Number('') === 0. parseFrontmatter() can produce '' in this case, which would reintroduce the “plausible zero” failure mode for malformed/hand-edited snapshots. Treat empty/whitespace-only strings as missing and return null.

    const num = (v) => {
      if (v == null) return null;
      const n = Number(v);
      return Number.isFinite(n) ? n : null;
    };

[pbakaus]

Addressed Copilot's suppressed empty-value finding in 357f3580. Empty and whitespace-only critique metrics now return null, with a regression covering empty, whitespace-only, and invalid values. Validation: node --test tests/context-signals.test.mjs passes 38/38, bun run build passes, the authoritative bun run test passes, and git diff --check is clean.

Response prepared with AI assistance under maintainer @pbakaus's standing automation authorization.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.