refactor(skills): make HVE behavior testing a final one-shot gate - #2821
refactor(skills): make HVE behavior testing a final one-shot gate#2821Allen Greaves (agreaves-ms) wants to merge 9 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2821 +/- ##
==========================================
+ Coverage 83.11% 83.23% +0.11%
==========================================
Files 183 184 +1
Lines 34068 34449 +381
Branches 25 25
==========================================
+ Hits 28317 28672 +355
- Misses 5748 5774 +26
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Eval Execution✅ Status: Passed
|
Katrien De Graeve (katriendg)
left a comment
Katrien De Graeve (katriendg)
left a comment
There was a problem hiding this comment.
Thanks for the rewrite, this should improve the cycles and repeated testing.
A few comments inline which may be worth reviewing.
| 3. Research only when needed. Use the `rpi-research` bridge in [stage-dispatch.md](stage-dispatch.md) for open-ended exploration, non-obvious reuse or extension discovery, and decision-critical evidence gaps. Do not substitute local discovery. | ||
| 4. Author the candidate. The lifecycle lead edits approved targets directly. Gather current requirements and findings before each coherent batch. Return to scope before a type change, artifact split, capability-surface change, or new support artifact outside the boundary. | ||
| 5. Validate the candidate. Run known non-mutating local checks, gather their complete in-scope finding set, and close those findings as a coherent batch before independent review. Do not invoke the behavior tester while validation remains open. | ||
| 6. Review and close static findings. Dispatch one fresh-context Medium-profile static review against the mechanically valid candidate. Apply its complete in-scope finding set in one correction batch, then run targeted static closure and every validation check affected by the corrections. If either remains open, stop Revise. If the assessed boundary changes, return to scope rather than claiming closure. |
There was a problem hiding this comment.
The extension reference still promises that a discovered, registered specialist reviewer is dispatched alongside generic static review. The rewritten lifecycle only defines one generic reviewer and contains no step that invokes an approved extension subagent by name.
A host can register Terraform Module Reviewer and have rpi-research discover it, but HVE Builder proceeds directly to the generic reviewer. The specialized checks never run even though the extension remains documented as supported.
Restore an explicit step that dispatches approved extension reviewers alongside the generic reviewer and merges their bounded findings before correction and candidate freeze, or remove the unsupported extension promise.
| 6. Review and close static findings. Dispatch one fresh-context Medium-profile static review against the mechanically valid candidate. Apply its complete in-scope finding set in one correction batch, then run targeted static closure and every validation check affected by the corrections. If either remains open, stop Revise. If the assessed boundary changes, return to scope rather than claiming closure. | |
| 6. Review and close static findings. Dispatch one fresh-context Medium-profile generic static review and every discovered, approved extension reviewer against the mechanically valid candidate. Merge their bounded findings, apply the complete in-scope finding set in one correction batch, then run targeted static closure and every validation check affected by the corrections. If either remains open, stop Revise. If the assessed boundary changes, return to scope rather than claiming closure. |
| | `Deferred` | A required stage or CI result is unavailable, behavior execution is Partial or Deferred, or the behavior verdict is Not available | | ||
| | `Revise` | Static findings remain, validation fails, behavior verdict is Revise, or an acceptance criterion is unmet | |
There was a problem hiding this comment.
The outcome table uses first-match ordering and maps every Partial behavior execution to Deferred before it considers a behavior verdict of Revise. If one scenario exposes an artifact defect and another fails after usable evidence is produced, the independent grader can return Revise while execution is Partial. HVE Builder then reports Deferred and sends the next owner toward an unchanged rerun instead of the actionable defect.
Give a graded Revise or Blocked verdict precedence over Partial execution. Map Partial to Deferred only when unavailable capability prevented completion and no actionable graded verdict determines the outcome.
| | `Deferred` | A required stage or CI result is unavailable, behavior execution is Partial or Deferred, or the behavior verdict is Not available | | |
| | `Revise` | Static findings remain, validation fails, behavior verdict is Revise, or an acceptance criterion is unmet | | |
| | `Revise` | Static findings remain, validation fails, behavior verdict is Revise, or an acceptance criterion is unmet | | |
| | `Deferred` | A required stage or CI result is unavailable, behavior execution is Partial or Deferred, or the behavior verdict is Not available | |
| pair based on the skill's behavior: | ||
| `Timing:` `after-candidate-freeze` | `before-candidate-freeze` | ||
| `Invocations:` `repeat-until-pass` | `zero-or-one` | ||
| `Revise:` `stop-and-defer` | `correct-and-retest` |
There was a problem hiding this comment.
The stimulus requires Revise: stop-and-defer, but the workflow contract keeps Revise and Deferred as distinct terminal outcomes. An actionable behavior finding resolves to Revise; Deferred is reserved for unavailable evidence or execution. The eval therefore rewards the wrong caller-facing disposition.
Replace stop-and-defer with stop-and-revise or equivalent Revise-preserving language in both the choices and regex.
| `Revise:` `stop-and-defer` | `correct-and-retest` | |
| `Revise:` `stop-and-revise` | `correct-and-retest` |
| - type: output-matches | ||
| name: final-candidate-decisions | ||
| config: | ||
| pattern: '(?i)^[ \t]*Timing:[ \t]*after-candidate-freeze[ \t]*\r?\n[ \t]*Invocations:[ \t]*zero-or-one[ \t]*\r?\n[ \t]*Revise:[ \t]*stop-and-defer[ \t]*(?:\r?\n)?(?![\s\S])' |
There was a problem hiding this comment.
Companion fix for the Revise disposition above: keep the grader pattern aligned with the corrected choice.
| pattern: '(?i)^[ \t]*Timing:[ \t]*after-candidate-freeze[ \t]*\r?\n[ \t]*Invocations:[ \t]*zero-or-one[ \t]*\r?\n[ \t]*Revise:[ \t]*stop-and-defer[ \t]*(?:\r?\n)?(?![\s\S])' | |
| pattern: '(?i)^[ \t]*Timing:[ \t]*after-candidate-freeze[ \t]*\r?\n[ \t]*Invocations:[ \t]*zero-or-one[ \t]*\r?\n[ \t]*Revise:[ \t]*stop-and-revise[ \t]*(?:\r?\n)?(?![\s\S])' |
| Design and grading run at the higher of Medium and the target profile, so the grader is never weaker than the executor it assesses. A Low target keeps design and grading at Medium; a High target raises both to High. This preserves independent semantic coverage and grading rather than pinning a fixed tier. | ||
| 1. Resolve targets, types, purpose, requirements, profile, requested fidelity, isolation and together sets, sandbox root, candidate revision, and report path. If no runtime behavior exists, write a supported skip report and return. | ||
| 2. Select fidelity through the methodology preconditions. Default to simulation. When requested native execution is unsupported or unsafe, use simulation only with caller acceptance; otherwise return Deferred with the rerun condition. | ||
| 3. Capture pre-run workspace state and create a unique sandbox containing `run-state.md`. Record the candidate revision, profile and model, fidelity, groupings, purpose, requirements, containment controls, and requirement map. |
There was a problem hiding this comment.
Flow step 3 requires run-state.md to contain a requirement map. Step 4 creates the scenario IDs and maps requirements to observable outcomes. A new run must therefore invent mappings before scenarios exist, leave a required field incomplete, or rewrite the earlier evidence contract silently.
Keep the requirement inventory in run-state.md and make test-design.md the sole owner of the requirement-to-scenario map.
| 3. Capture pre-run workspace state and create a unique sandbox containing `run-state.md`. Record the candidate revision, profile and model, fidelity, groupings, purpose, requirements, containment controls, and requirement map. | |
| 3. Capture pre-run workspace state and create a unique sandbox containing `run-state.md`. Record the candidate revision, profile and model, fidelity, groupings, purpose, requirements, and containment controls. |
| | # | Action | Mapped dimension | Artifact | Profile | Evidence class | Severity | Evidence | Resolving change | | ||
| |---|--------|------------------|----------|---------|----------------|----------|----------|------------------| | ||
| |---|---|---|---|---|---|---|---|---| |
There was a problem hiding this comment.
The reference requires every finding to record both fidelity and evidence class, but the canonical Findings table has only Evidence class. These are not equivalent: native or simulation identifies how the target ran, while observed, simulated, or emulated classifies the evidence.
Add a Fidelity column to the canonical Findings table and preserve it separately from Evidence class.
| | # | Action | Mapped dimension | Artifact | Profile | Evidence class | Severity | Evidence | Resolving change | | |
| |---|--------|------------------|----------|---------|----------------|----------|----------|------------------| | |
| |---|---|---|---|---|---|---|---|---| | |
| | # | Action | Mapped dimension | Artifact | Profile | Fidelity | Evidence class | Severity | Evidence | Resolving change | | |
| |---|---|---|---|---|---|---|---|---|---| |
| 3. Run fresh-context static review and one behavior gate with route-specific execution: Major mutations and behavior-bearing review targets execute testing, while eligible no-runtime review targets and Minor or Medium mutations are satisfied-and-skipped | ||
| 3. Complete all known edits, fresh-context static review, and local validation before freezing the candidate and resolving one final behavior gate. Major mutations and behavior-bearing review targets invoke HVE Builder Tester at most once; eligible no-runtime review targets and Minor or Medium mutations are satisfied-and-skipped. A behavior finding ends the current run and becomes input to a later invocation rather than a same-run edit and retest | ||
| 4. Keep known target files and caller-supplied canonical references as bounded lifecycle reads; activate `rpi-research` for open-ended exploration and decision-critical research | ||
| 5. Run non-mutating host validation and resolve one overall outcome |
There was a problem hiding this comment.
Step 3 places local validation before candidate freeze and the final behavior gate. Step 5 then tells readers to run non-mutating host validation afterward. The workflow contract forbids validation after tester dispatch in the same run, so the public sequence can invalidate the frozen-boundary evidence.
Move all host validation before freeze and make the final step outcome resolution only.
| 5. Run non-mutating host validation and resolve one overall outcome | |
| 5. Resolve one overall outcome |
| | `prompts.eval.yaml` | 3p | Advisory | 51 | `behavior-conformance` | Active (Phase 9) | | ||
| | `instructions.eval.yaml` | 3i | Advisory | 62 | `behavior-conformance` | Active (Phase 11) | | ||
| | `skill-behavior.eval.yaml` | 3s | Advisory | 219 | `behavior-conformance` | Active (Phase 13) | | ||
| | `instructions.eval.yaml` | 3i | Advisory | 64 | `behavior-conformance` | Active (Phase 11) | |
There was a problem hiding this comment.
The README reports 64 stimuli, but direct counts show 65 total entries: 63 instruction-tagged plus two backlog-management entries.
| | `instructions.eval.yaml` | 3i | Advisory | 64 | `behavior-conformance` | Active (Phase 11) | | |
| | `instructions.eval.yaml` | 3i | Advisory | 65 | `behavior-conformance` | Active (Phase 11) | |
| The maintained `prompts.eval.yaml` inventory contains 51 stimuli across 48 prompt subjects. Coverage includes RPI orchestration, security review and planning, Design Thinking, Git and pull request operations, evaluation authoring, and VEX workflows. Backlog and work-item coverage moved to `skill-behavior.eval.yaml` when those workflows became skills. | ||
|
|
||
| The maintained `instructions.eval.yaml` inventory contains 62 stimuli across 46 instruction subjects. Coverage spans: | ||
| The maintained `instructions.eval.yaml` inventory contains 64 stimuli: 62 instruction-tagged stimuli across 46 instruction subjects, plus two `backlog-management` skill stimuli. Coverage spans: |
There was a problem hiding this comment.
Keep the prose inventory aligned with the corrected table and direct counts.
| The maintained `instructions.eval.yaml` inventory contains 64 stimuli: 62 instruction-tagged stimuli across 46 instruction subjects, plus two `backlog-management` skill stimuli. Coverage spans: | |
| The maintained `instructions.eval.yaml` inventory contains 65 stimuli: 63 instruction-tagged stimuli across 46 instruction subjects, plus two `backlog-management` skill stimuli. Coverage spans: |
- converge edits and validation before behavior testing - simplify tester execution and always-loaded guidance - add final-candidate conformance coverage ♻️ - Generated by Copilot
- reject contradictory timing, invocation, and revise outputs - align declared stimulus total with the parsed inventory 🧪 - Generated by Copilot
- constrain output to three opposing choices - prevent partial grader scores from passing 🧪 - Generated by Copilot
- document current deterministic and LLM grader use - distinguish eval graders from hygiene checks 📝 - Generated by Copilot
- report all 64 instruction stimuli - align skill subject count with source tags 📝 - Generated by Copilot
📝 - Generated by Copilot
🔧 - Generated by Copilot
📝 - Generated by Copilot
9ff4f67 to
f5b2d5e
Compare
Bill Berry (WilliamBerryiii)
left a comment
Bill Berry (WilliamBerryiii)
left a comment
There was a problem hiding this comment.
Thank you for consolidating HVE Builder around a frozen final candidate and one terminal behavior gate. The comprehensive review found two blocking contract issues before this can merge. Native fidelity can currently be authorized without verifying the target's effective tool and external-action authority, so its read-only or contained precondition is not enforceable. The tester also has no contract-valid recoverable state when execution completes but independent grading is temporarily unavailable. Please address those blockers, then bind the frozen candidate to its test evidence, restore isolation for scenario design, and add tester behavior evidence. For specialist integration, please add a generic pre-freeze assurance envelope so domain-owned RAI, Privacy, and Accessibility adapters can reuse adequate accepted evidence for unchanged obligations, run targeted checks for bounded impact, and require full review or defer when impact is material or indeterminate. The branch should also be updated from main, required checklist evidence completed, and open review threads resolved before re-review.
|
Addressed comments in #2843 closing this PR as that one replaces this one. |
5 participants
Description
This PR changed HVE Builder and HVE Builder Tester so behavior testing occurs once against a frozen final candidate instead of recurring during small correction loops. It also reduced always-loaded authoring guidance, retained independent static review and evidence grading, and added advisory conformance coverage for the final-gate decisions.
Final-candidate lifecycle
One-run tester and focused guidance
Regression and documentation
after-candidate-freeze,zero-or-one, andstop-and-defer.Related Issue(s)
None.
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
hve-builderand addressed all actionable findings.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md).github/skills/*/SKILL.md).github/hooks/*/*.json)evals/)Other:
.ps1,.sh,.py)Sample Prompts (for AI Artifact Contributions)
User Request:
Improve an existing Copilot skill, gather all known changes before testing, and validate the final behavior without repeatedly rerunning the behavior tester.
Execution Flow:
Satisfied-and-skipped; a Major or behavior-bearing target invoked HVE Builder Tester once.Output Artifacts:
Success Indicators:
For detailed contribution requirements, see:
Testing
The branch was rebased onto current
main, and both npm package roots were restored from their lockfiles.Passed local validation:
npm run validate:docs(10 Jest suites, 78 tests, and 11 Mermaid accessibility tests)npm run spell-checkvalidate:localconstituent check after the link step, including Python lint across 17 projects after their locked uv environments were provisionednpm run validate:localand the standalonenpm run lint:md-linksstopped on six external URLs in two files unchanged frommain. The changed-file link scope used by hosted PR CI passed. HVE Builder Tester was not run during this change. Hosted behavior-conformance evidence remains advisory CI output.Checklist
Required Checks
AI Artifact Contributions
hve-builderreview mode to review contributionhve-builderreviewRequired Local Checks
The following local-safe validation commands must pass before merging:
npm run validate:local(inherited full-link baseline; all other constituent checks passed)npm run validate:docsnpm run spell-checknpm run lint:md-links(inherited full-link baseline; hosted PR scope passed)Security Considerations
Additional Notes
main; one generated instruction-index date conflict was resolved to 2026-08-31.