Conversation
Enforce meaningful verifier revisions and severity observability, preserve verified low-confidence findings and publication-aware notes, and attach bounded related promotion signals without changing selection.
🧞 Codegenie ReviewReviewed 70 of 88 hunks (18 skipped, 0 failed; no budget stop, no degraded planning). One verified finding: in CoverageReviewed 70/88 hunks.
|
There was a problem hiding this comment.
🧞 Codegenie Review
Reviewed 70 of 88 hunks (18 skipped, 0 failed; no budget stop, no degraded planning). One verified finding: in src/pipeline/uncertainty-promotion.ts, the related-signal association gate requires overlap on both files and symbols, so eligible file-only (or symbol-only) promotion sources can never be represented as related signals. Reported inline as low severity with a suggested fix and test. No other verified issues; verifier fail-closed logic, publication-aware human-attention adjudication, and eval loss labeling changes had no confirmed defects in the reviewed hunks.
Reviewed 70/88 hunks.
Incomplete work: skipped 18.
🙋 Needs human attention:
- Does SubmitVerificationVerdictSchema's reordered property emission (behaviorChange/intentEvidence now before finalFinding/revisedAnchor) matter for any provider that keys on property order or for cached tool-schema fingerprints beyond SCHEMA_VERSIONS?
- Are downstream consumers/reports of EvalFindingLoss.noteGroupExisted defined so that an internal-only note group is not counted as a user-visible NOTE outcome (i.e. only surfacedAsNote drives recall/loss credit)?
- Does the verifier repair candidate summary actually emit changedCode/failureMode/whyThisMatters/verification with the exact fakeFinding() values ("bad"/"matters"/"verified"), so these new toContain assertions match the production serialization format?
- Are the numeric related-signal bounds in spec line 601 (at most eight signals, three shared attention terms, Jaccard >= 0.24) the same constants used in the uncertainty-promotion lane implementation?
- Are there existing tests covering the case where fallback groups exceed MAX_HUMAN_ATTENTION_NOTES so omittedFallbackCount is non-zero?
- Additional unresolved notes suppressed: 4
— codegenie v0.5.3 (1824056dc9) · View Workflow Job
| if (riskProfile(source.source).category !== riskProfile(selected.source).category || | ||
| source.promotionClass !== selected.promotionClass || | ||
| !normalizedValuesOverlap(source.source.files, selected.source.files) || | ||
| !normalizedValuesOverlap(source.source.symbols, selected.source.symbols)) { |
There was a problem hiding this comment.
relatedPromotionAssociation requires overlap on both files and symbols, so an eligible promotion source that lists only files (or only symbols) can never be represented as a related signal.
if (riskProfile(source.source).category !== riskProfile(selected.source).category ||
source.promotionClass !== selected.promotionClass ||
!normalizedValuesOverlap(source.source.files, selected.source.files) ||
!normalizedValuesOverlap(source.source.symbols, selected.source.symbols)) {
return undefined;
}
...
function normalizedValuesOverlap(left: string[], right: string[]): boolean {
const leftValues = new Set(left.map(normalize).filter(Boolean));
return right.some((value) => leftValues.has(normalize(value)));
}normalizedValuesOverlap is right.some(...) over a Set built from left, so it returns false whenever either side is empty. Eligibility, however, only rejects a source when both dimensions are empty:
if (source.files.length === 0 && source.symbols.length === 0) {
return { eligible: false, reason: "no_concrete_file_or_symbol" };
}and pointsAtDistinctScope explicitly handles symbols.length === 0 by qualifying on files alone, confirming file-only sources are a reachable class rather than hypothetical.
Impact: For a file-only unselected source, the symbol clause is false against every selected item, so relatedPromotionAssociation returns undefined for all pairs and the source is always recorded as promotion_lane_limited:
decisions.push(association === undefined
? baseDecision(unselectedItem.source, false, "promotion_lane_limited", promotionDecisionMetadata(unselectedItem))
: {
...baseDecision(unselectedItem.source, false, "represented_as_related_signal", promotionDecisionMetadata(unselectedItem)),The selected candidate never receives relatedSignals or crossPacketRelatedCount even when the file scope and normalized question are identical, so that evidence is dropped from published provenance and the new counters undercount represented related signals. The symmetric symbols-only case fails the same way on the files clause. This fails closed rather than emitting wrong output, hence low severity.
The commit body states the change should "attach bounded related promotion signals without changing selection", and no repo test asserts relatedSignals/crossPacketRelatedCount for a symbols: [] source, so the narrow conjunctive gate changes the coverage contract in a way that needs confirmation against the intended boundary.
Suggested fix: treat an empty dimension as non-discriminating — compare a dimension only when both sides populate it, and accept overlap on either:
const filesComparable = source.source.files.length > 0 && selected.source.files.length > 0;
const symbolsComparable = source.source.symbols.length > 0 && selected.source.symbols.length > 0;
const scopeOverlap =
(filesComparable && normalizedValuesOverlap(source.source.files, selected.source.files)) ||
(symbolsComparable && normalizedValuesOverlap(source.source.symbols, selected.source.symbols));
if (!scopeOverlap) {
return undefined;
}Suggested test: one selected and one unselected eligible source that share a file, have an identical normalized question, and both carry symbols: []; assert the selected promoted candidate carries relatedSignals (and crossPacketRelatedCount when packets differ) and the unselected source is recorded as represented_as_related_signal rather than promotion_lane_limited.
Summary
This PR strengthens the verifier and publication pipeline around five generic contracts discovered through repeated live evals:
reviseverdicts, canonicalize legacykeepverdicts that carry revisions, and fail closed on semantically incomplete verificationThe follow-up Stage 9 hardening replaces the provider-fragile root union with a provider-safe flat submit schema, distinguishes empty primary submissions, uses a bounded evidence-bearing stateless repair prompt, and prevents a repaired response from inventing adjudication state for an empty primary verdict.
The package is bumped to
0.5.4, including the documented GitHub Action references.Why
Repeated evals exposed several independent failure modes: provider schema incompatibility, empty or payload-free verifier submissions, verified low-confidence findings disappearing at publication, rejected/published questions leaking into human-attention notes, related uncertainty signals being lost at the promotion limit, and insufficient visibility into verifier severity changes.
These changes encode pipeline-wide invariants rather than run-specific rules. They do not add eval-id checks, new selection thresholds, or candidate-specific exceptions.
Implementation notes
{}submissions remain incomplete even if a repair returns a valid verdict; repair cannot reconstruct missing primary adjudication state.reviserequiresfinalFindingorrevisedAnchor; missing payloads are persisted asrevise_without_revision_payload.Validation
pnpm run checkpnpm test— 793/793 tests passing across 40 filespnpm buildgit diff --check49f4645bruns 61 and 62 passed0c4d5213runs 69 and 70 passedThe two independent eval cases repeatedly preserved their required findings and prohibited-finding guards. Across these post-fix runs, Stage 9 avoided the earlier empty-object/schema collapse while continuing to reject or narrow unsupported findings.
Known follow-ups
requiredEvidencePresent: false, and a published resolution can miss a semantically identical note whose primary file differs. Follow-up should use exact packet/candidate lineage and strong symbol/predicate identity without broad fuzzy suppression.revisewithout a payload. The new semantic defense correctly marked it incomplete and prevented publication, but the repair-path wiring should be reviewed so this shape can receive one bounded retry.severityAtLeastmatching should be audited to ensure it uses final post-verification severity rather than a merged pre-verification value.These follow-ups do not affect the required expectation passes in the validated runs, but they are recorded here so the green eval status is not overstated.