What happened
On PR #722, the author introduced 6 new YAML measurement manifests under eval/measurements/ -- a new config file category with no pre-existing CI or pre-commit validation. The human reviewer flagged this on the first review sweep (Aug 10), noting that: (1) the repo has lint-agent-docs for harness/*.yaml and eval/lint-cases.sh for eval cases, but neither covers eval/measurements/*.yaml, (2) no CI workflow step validates these files, and (3) the consumer code in fullsend's ScoreTrace silently skips unknown scorer values via a default: switch case, meaning a typo would silently produce zero measurements in production. The review agent approved 3 times (Aug 10--13) without flagging this gap. The human's finding was the catalyst for the entire eval/lint-measurements.sh script (395 lines of validation logic plus 301 lines of tests).
What could go better
The review agent's correctness sub-agent should detect when a PR introduces new config file types that fall outside all existing validation infrastructure. The first two checks are mechanically detectable: (1) identify new files whose paths don't match any files: pattern in .pre-commit-config.yaml hooks, and (2) check whether any CI workflow step explicitly validates files at that path. A third check (whether the consumer code uses silent-skip semantics for unknown values) requires cross-repo reasoning and is harder. Confidence is high that checks 1 and 2 are feasible and would have caught this -- the correctness sub-agent already has access to .pre-commit-config.yaml and .github/workflows/ in the repo. This is distinct from #394 (CI workflow file correctness), #397 (test coverage estimation for code), and #562 (config-loader defensive validation).
Proposed change
Add guidance to the correctness sub-agent definition in skills/pr-review/sub-agents/ instructing it to: when a PR introduces files under a new directory path or file pattern that doesn't match any existing pre-commit hook's files: regex or CI workflow step's path filter, flag a LOW finding noting the validation gap. Escalate to MEDIUM when the consumer code (if identifiable from the PR body or README) uses silent-skip or fallback semantics for unknown values, since typos would silently no-op in production. The check should cross-reference .pre-commit-config.yaml hooks and .github/workflows/ steps in the repo to determine existing validation coverage.
Validation criteria
On the next PR in this repository that adds a new config file category without corresponding lint or CI validation, the review agent should produce at least a LOW finding noting the missing validation coverage. Measurable over the next 3 PRs that introduce new config file types.
Generated by retro agent from #722
What happened
On PR #722, the author introduced 6 new YAML measurement manifests under
eval/measurements/-- a new config file category with no pre-existing CI or pre-commit validation. The human reviewer flagged this on the first review sweep (Aug 10), noting that: (1) the repo haslint-agent-docsforharness/*.yamlandeval/lint-cases.shfor eval cases, but neither coverseval/measurements/*.yaml, (2) no CI workflow step validates these files, and (3) the consumer code in fullsend'sScoreTracesilently skips unknown scorer values via adefault:switch case, meaning a typo would silently produce zero measurements in production. The review agent approved 3 times (Aug 10--13) without flagging this gap. The human's finding was the catalyst for the entireeval/lint-measurements.shscript (395 lines of validation logic plus 301 lines of tests).What could go better
The review agent's correctness sub-agent should detect when a PR introduces new config file types that fall outside all existing validation infrastructure. The first two checks are mechanically detectable: (1) identify new files whose paths don't match any
files:pattern in.pre-commit-config.yamlhooks, and (2) check whether any CI workflow step explicitly validates files at that path. A third check (whether the consumer code uses silent-skip semantics for unknown values) requires cross-repo reasoning and is harder. Confidence is high that checks 1 and 2 are feasible and would have caught this -- the correctness sub-agent already has access to.pre-commit-config.yamland.github/workflows/in the repo. This is distinct from #394 (CI workflow file correctness), #397 (test coverage estimation for code), and #562 (config-loader defensive validation).Proposed change
Add guidance to the correctness sub-agent definition in
skills/pr-review/sub-agents/instructing it to: when a PR introduces files under a new directory path or file pattern that doesn't match any existing pre-commit hook'sfiles:regex or CI workflow step's path filter, flag a LOW finding noting the validation gap. Escalate to MEDIUM when the consumer code (if identifiable from the PR body or README) uses silent-skip or fallback semantics for unknown values, since typos would silently no-op in production. The check should cross-reference.pre-commit-config.yamlhooks and.github/workflows/steps in the repo to determine existing validation coverage.Validation criteria
On the next PR in this repository that adds a new config file category without corresponding lint or CI validation, the review agent should produce at least a LOW finding noting the missing validation coverage. Measurable over the next 3 PRs that introduce new config file types.
Generated by retro agent from #722