Skip to content

Commit 5f57fc3

Browse files
author
Akkari
committed
fix: resolve validation findings from 4-critic self-review
CRITICAL fix: - {PRESCREEN_EVIDENCE} now injected for security AND code_hygiene critics (was incorrectly scoped to security only, breaking code_hygiene's pre-screen integration design) HIGH fixes: - Document all 10 pre-screen checks (PS-001–PS-010) including PS-004/005/009/010 - Add --dispatch parameter to Parameters table - Add error handlers: corrupt rubric JSON, invalid critic JSON response - Fix verdict PASS condition: 'No findings or only INFO' (was undefined 'FAIL') - Add DEGRADED/PARTIAL verdict behavior documentation - Add evidence_tool to Step 4a validation requirements MEDIUM fixes: - Fix Quick Reference: 'task agents, sequential by default' (was 'parallel') - Align critic dispatch order between diagram and progress messages - Normalize 'Security Critic' naming in code_hygiene.yaml (was mixed CamelCase) - Mark RELATIONSHIPS parameter and escalation rule as 'not yet ported' - Sanitize error message templates (rubric names only, not full path listing)
1 parent 1cda7e4 commit 5f57fc3

3 files changed

Lines changed: 30 additions & 22 deletions

File tree

ports/copilot-cli/SKILL.md

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ USER INVOCATION → Parse parameters
1313
1414
├─ PRE-SCREEN: Run quorum-prescreen.py (deterministic, <5s)
1515
16-
├─ CRITIC DISPATCH (parallel task agents):
16+
├─ CRITIC DISPATCH (task agents, sequential by default):
1717
│ ├─ Correctness Critic → factual accuracy, logical consistency
1818
│ ├─ Completeness Critic → coverage gaps, missing requirements
1919
│ ├─ Security Critic → framework-grounded security analysis
@@ -32,8 +32,9 @@ USER INVOCATION → Parse parameters
3232
|-----------|----------|---------|-------------|
3333
| `TARGET` | Yes || Path to the artifact to validate |
3434
| `RUBRIC` | No | Auto-detect from file extension | Rubric name or path to rubric JSON |
35-
| `DEPTH` | No | `standard` | `quick` (1 critic) / `standard` (3 critics) / `thorough` (3 + learning review) |
36-
| `RELATIONSHIPS` | No || Path to `quorum-relationships.yaml` for cross-artifact checks |
35+
| `DEPTH` | No | `standard` | `quick` / `standard` / `thorough` (see Depth Profiles below) |
36+
| `RELATIONSHIPS` | No || Path to `quorum-relationships.yaml` for cross-artifact checks (not yet ported; reserved for future use) |
37+
| `--dispatch` | No | `lightweight` | Dispatch tier: `lightweight` (sequential), `standard` (2 concurrent), `performance` (4 concurrent) |
3738

3839
**Auto-detection mapping:**
3940
- `.py``python-code`
@@ -79,13 +80,17 @@ Run the deterministic pre-screen before any critic dispatch:
7980
python3 quorum-prescreen.py "{TARGET}" --output json
8081
```
8182

82-
The pre-screen runs 10 regex-based checks (PS-001 through PS-010):
83+
The pre-screen runs regex-based checks including:
8384
- PS-001: Hardcoded paths
8485
- PS-002: Credential patterns
8586
- PS-003: PII patterns
87+
- PS-004: JSON validity
88+
- PS-005: YAML validity
8689
- PS-006: Python syntax
8790
- PS-007: Broken markdown links
8891
- PS-008: TODO markers
92+
- PS-009: Whitespace issues
93+
- PS-010: Empty file detection
8994

9095
Capture the JSON output. This becomes `{PRESCREEN_EVIDENCE}` injected into critic prompts.
9196

@@ -106,7 +111,7 @@ Launch all 4 critics. Each receives:
106111
- Any mandatory known-issue patterns
107112

108113
### At `thorough` depth
109-
Same as standard (4 critics), plus:
114+
Same as standard, plus:
110115
- All known-issue patterns (not just mandatory) injected into critic prompts
111116
- Model tier upgraded to Tier 1 (Opus-class) for all critics
112117

@@ -117,7 +122,7 @@ For each critic, construct the prompt by filling the critic YAML's `prompt_templ
117122
1. `{ARTIFACT_TEXT}` ← full artifact content
118123
2. `{RUBRIC_NAME}`, `{RUBRIC_VERSION}`, `{RUBRIC_DOMAIN}` ← from rubric JSON
119124
3. `{CRITERIA_TEXT}` ← formatted criteria list (filter by critic's `rubric_keywords`, or all for completeness)
120-
4. `{PRESCREEN_EVIDENCE}` ← pre-screen JSON output (security critic only; empty for others)
125+
4. `{PRESCREEN_EVIDENCE}` ← pre-screen JSON output (security and code_hygiene critics; empty for correctness and completeness)
121126
5. `{EXTRA_CONTEXT}` ← mandatory known-issue patterns + any additional context
122127

123128
**Dispatch via `task` tool:**
@@ -130,8 +135,8 @@ Each critic is dispatched as a task agent. The system prompt comes from the crit
130135

131136
**Progress indicators:** After dispatching each critic, inform the user:
132137
- "🔍 Correctness critic dispatched (1 of 4)..."
133-
- "🔍 Security critic dispatched (2 of 4)..."
134-
- "🔍 Completeness critic dispatched (3 of 4)..."
138+
- "🔍 Completeness critic dispatched (2 of 4)..."
139+
- "🔍 Security critic dispatched (3 of 4)..."
135140
- "🔍 Code Hygiene critic dispatched (4 of 4)..."
136141
- "⏳ Waiting for critic results..."
137142

@@ -148,6 +153,7 @@ After all critics return, aggregate findings into a verdict.
148153
Parse each critic's JSON output. Validate that every finding has:
149154
- `severity` (CRITICAL/HIGH/MEDIUM/LOW/INFO)
150155
- `description` (non-empty)
156+
- `evidence_tool` (non-empty — how the finding was verified)
151157
- `evidence_result` (non-empty — reject findings without evidence)
152158

153159
**Reject ungrounded findings.** If a finding lacks `evidence_result`, discard it and log: "Finding rejected: no evidence provided."
@@ -169,9 +175,9 @@ Apply rules from `verdict-rules.yaml` in order:
169175
| 3+ HIGH findings | **REVISE** |
170176
| Any HIGH (fewer than 3) | **PASS_WITH_NOTES** |
171177
| Only MEDIUM/LOW | **PASS_WITH_NOTES** |
172-
| No findings with evidence | **PASS** |
178+
| No findings (or only INFO) | **PASS** |
173179

174-
**Escalation:** If cross-artifact relationship checks found HIGH or CRITICAL issues, escalate verdict by one level (PASS → PASS_WITH_NOTES, PASS_WITH_NOTES → REVISE).
180+
**Escalation:** If cross-artifact relationship checks found HIGH or CRITICAL issues, escalate verdict by one level (PASS → PASS_WITH_NOTES, PASS_WITH_NOTES → REVISE). Note: relationship checks are not yet ported; this rule is reserved for future use.
175181

176182
**REJECT** is never assigned automatically — it requires your supervisor judgment that the artifact is fundamentally unsalvageable.
177183

@@ -247,10 +253,12 @@ After verdict, update `learning/known_issues.json`:
247253
| Failure | Action |
248254
|---------|--------|
249255
| Target not found | Abort: "❌ Target file not found: {path}" |
250-
| Rubric not found | Abort: "❌ Rubric not found: {name}. Available: {list}" |
256+
| Rubric not found | Abort: "❌ Rubric not found: {name}. Available rubrics: [list names only]" |
257+
| Rubric JSON malformed | Abort: "❌ Rubric parse error: {name}. Verify JSON syntax." |
258+
| Critic returns invalid JSON | Treat as critic failure (DEGRADED). Log warning, proceed with remaining critics. |
251259
| Pre-screen script missing | Warn, continue without pre-screen |
252-
| 1-2 critics fail/time out | DEGRADED: produce verdict from remaining critics |
253-
| 3 critics fail | PARTIAL: produce verdict from sole remaining critic |
260+
| Some critics fail/time out | DEGRADED: produce verdict from remaining critics. Apply standard verdict rules to available findings. Tag output with "⚠️ DEGRADED: N of M critics returned." |
261+
| All but one critic fail | PARTIAL: produce verdict from sole remaining critic. Tag output with "⚠️ PARTIAL." Verdict reflects only that critic's coverage. |
254262
| All critics fail | Abort: "❌ QUORUM_FAILED: All critics failed" |
255263
| Finding lacks evidence | Reject finding silently, count in summary |
256264
| Known issues file corrupted | Warn, continue without learning memory |
@@ -294,4 +302,4 @@ cp -r quorum-copilot-skill ~/.copilot/skills/quorum
294302

295303
**This is not** the full Quorum reference implementation. The CLI version has additional capabilities (batch processing, fix loops, cost tracking, tester verification). This skill covers the highest-value portion: deterministic pre-screen → parallel critic dispatch → evidence-grounded findings → deterministic verdict.
296304

297-
**Capability coverage:** ~75% of reference implementation. Full 4-critic suite (correctness, completeness, security, code hygiene). Missing: tester (L1/L2 verification), fixer (automated remediation), batch mode, cost tracking, structured output artifacts. These are planned for future releases.
305+
**Capability coverage:** ~75% of reference implementation. Includes all four core evaluation critics. Not yet ported: L1/L2 verification, automated remediation, batch mode, cost tracking, structured output artifacts. These are planned for future releases.

ports/copilot-cli/critics/code_hygiene.yaml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ model_tier: 2 # Sonnet-class for standard depth; Opus for thorough
1616
# ─── DELEGATION BOUNDARY ─────────────────────────────────────────────────────
1717
# This critic flags security-adjacent patterns for traceability and DELEGATES
1818
# security severity assessment to the Security Critic. It does NOT assign
19-
# HIGH/CRITICAL to these patterns — that is SecurityCritic's domain.
19+
# HIGH/CRITICAL to these patterns — that is Security Critic's domain.
2020

2121
delegation:
2222
delegates_to:
@@ -28,7 +28,7 @@ delegation:
2828
- "Unsanitized user input in prompt construction (AP-01)"
2929
finding_template: >
3030
Code hygiene concern: {pattern} is difficult to audit and test.
31-
Security severity and exploitability assessment delegated to SecurityCritic.
31+
Security severity and exploitability assessment delegated to Security Critic.
3232
max_severity_from_this_critic: MEDIUM
3333

3434
# ─── SYSTEM PROMPT ───────────────────────────────────────────────────────────
@@ -82,7 +82,7 @@ system_prompt: |
8282
━━━ DO NOT ━━━
8383
- Re-flag PASSED pre-screen checks
8484
- Report auto-fixable formatting issues (indentation, line length, whitespace)
85-
- Assign security severity to credential/injection patterns — delegate to SecurityCritic
85+
- Assign security severity to credential/injection patterns — delegate to Security Critic
8686
- Invent code quotes — only cite text that appears verbatim in the artifact
8787
- Flag the same issue in multiple findings
8888
@@ -230,7 +230,7 @@ categories:
230230
description: >
231231
Environment variable usage (no plaintext fallback defaults), secrets in log
232232
output at any level, secrets in exception messages, .env files excluded from VCS.
233-
NOTE: Flag pattern only — delegate security assessment to SecurityCritic.
233+
NOTE: Flag pattern only — delegate security assessment to Security Critic.
234234
235235
- id: AP-05
236236
name: Timeout & Retry Logic
@@ -257,8 +257,8 @@ prescreen_integration:
257257
clean — do NOT re-flag. SKIP checks are not applicable.
258258
check_mapping:
259259
PS-001: { relates_to: [CAT-11, AP-04], action: "Assess semantic impact of hardcoded paths" }
260-
PS-002: { relates_to: [AP-04], action: "Flag hygiene concern, delegate security to SecurityCritic" }
261-
PS-003: { relates_to: [AP-04], action: "Flag hygiene concern, delegate security to SecurityCritic" }
260+
PS-002: { relates_to: [AP-04], action: "Flag hygiene concern, delegate security to Security Critic" }
261+
PS-003: { relates_to: [AP-04], action: "Flag hygiene concern, delegate security to Security Critic" }
262262
PS-006: { relates_to: [CAT-01], action: "Assess semantic impact of syntax issue" }
263263
PS-007: { relates_to: [CAT-06], action: "Assess documentation impact of broken links" }
264264
PS-008: { relates_to: [CAT-06], action: "Assess whether TODOs indicate incomplete implementation" }
@@ -308,7 +308,7 @@ prompt_template: |
308308
309309
Delegation: For eval()/exec(), Invoke-Expression, hardcoded credentials, or
310310
unsanitized prompt input — flag the hygiene concern and state that security
311-
assessment is delegated to SecurityCritic. Assign LOW/MEDIUM only.
311+
assessment is delegated to Security Critic. Assign LOW/MEDIUM only.
312312
313313
For each finding:
314314
1. Quote the specific code excerpt OR cite pre-screen check ID

ports/copilot-cli/verdict-rules.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ rules:
3737
- condition: "Only MEDIUM and/or LOW findings"
3838
verdict: PASS_WITH_NOTES
3939

40-
- condition: "No FAIL findings"
40+
- condition: "No findings, or only INFO-severity findings"
4141
verdict: PASS
4242

4343
# ─── ESCALATION ──────────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)