Skip to content

Skill Eval: actionable, revision-bound review guidance for skill and knowledge PRs #215

Closed

Description

@Dong1017

Goal

Make Skill Eval answer a practical review question: what does the current evidence support merging, what needs correction, and what remains untested? Deliver advisory, evidence-linked guidance—not automatic approval or a new required paid CI gate.

Refs #88 and #101 (accepted workflow foundation), #91 (coverage inventory), and #179 (evidence-integrity corrections). This is a bounded follow-on, not a reopening of their accepted scope or a transfer of existing ownership.

Starting point and gap

#88 acceptance established automatic addition/update comparisons and retained PR feedback. Each real comparison exercised one fixed holdout; a neutral update was valid. That acceptance explicitly did not establish whole-library efficacy or subject-PR merge readiness.

The current mapping names inference-benchmark-design and audit-fixed-workload-claim, both using the audit case file. The publisher reports outcomes and coverage gaps but does not explain their consequence for a PR decision. PLANNED, skipped execution, expired evidence and results for old revisions are not quality passes. The workflow's path filter does not cover every knowledge-only path.

Reuse the existing architecture

Verified source surfaces: .github/workflows/skill-eval-pr.yml, tools/skill_eval_pr.py, mapping, comparison controller, and usage guide. Reconcile these against the implementation's exact starting SHA.

flowchart LR
    A["PR scope and mapped cases"] --> B["Existing frozen plan"]
    B --> G["Owner-enabled label + eligibility + remaining PR budget"]
    G --> C["Revision-bound authorization and paired execution"]
    C --> D["Retained evidence and trusted regrade"]
    D --> E["Advisory review guidance and explicit gaps"]
    E --> F["Independent review and authorized merge decision"]
Loading

Keep prepare/authorize/execute/pack/publish, skill_eval_compare, the maintained mapping and one identifiable PR comment. Extend trusted report derivation and presentation; do not build a second runner, registry or dashboard. Preserve execution/publication credential separation and publication-only retry.

Bounded delivery

Phase 1 — Make existing mapped PR evidence decision-useful

Use #159 and #160 as the initial addition/update subjects, subject to their current state and exact revisions. If either is no longer suitable, record a specific replacement before using it.

  • Show changed scope, selected cases, uncovered/unsupported paths, both compared SHAs, trusted implementation, plan/exam identity and retained evidence availability.
  • Derive a concise advisory disposition with reasons and next actions: address observed regression, obtain missing evidence, or covered evidence supports proceeding to independent review. Existing failures remain visible. A fix does not cancel a regression; correct→correct is not incremental benefit.
  • Never issue a blanket “approved” verdict. Uncovered scope needs a case or explicit reviewer disposition; no arbitrary all-files-covered requirement. Content/source review, authorship admission and merge authority stay separate.
  • A new base/head or relevant plan input invalidates applicability. Bind guidance to the exact existing plan identity; do not silently reuse old runs. Missing, unexecuted, stale, expired, erroneous or incomplete evidence must not become positive quality evidence.
  • Clarify the documented entry path for knowledge-only PRs: outside the supported scope must be visible, not mistaken for an eval pass. Any automatic-trigger expansion must name its bounded paths and receive focused tests; do not blanket-trigger all repository files.

Phase 2 — One justified coverage extension, not a library campaign

After Phase 1, select at most one currently unmapped task family from an actual pending skill/knowledge PR. Record the target PR, intended behavior, source/skill paths, independent expected outcomes, negative/boundary cases and supported grader before implementation. Do not force an unrelated task through the audit grader. If a new trusted task/grader adapter is necessary, record that bounded scope first; wider coverage remains separate under #91.

Owner-approved extension — label-gated automatic paid execution

The owner requested automatic paid evaluation for suitable PRs, with a USD 200 cumulative budget per PR. A dedicated opt-in label is the proposed control (for example, skill-eval:execute; final name to be confirmed against existing labels). This records the delivery requirement; the switch is not implemented or enabled by this issue edit.

The following are proposed implementation and acceptance constraints for that requirement, not a claim that the owner prescribed this exact design:

  • Define suitability using supported changed scope, meaningful mapped cases, a trusted grader/provider configuration, and a current frozen plan. A label alone does not make an unsupported PR executable or quality-approved.
  • Only an owner-authorized operator may enable the switch; verify the label action and current permission through trusted repository metadata. Do not treat arbitrary PR text, an author's request, or an untrusted event payload as execution authorization. Keep secrets away from untrusted PR code.
  • Once enabled, automatically prepare and authorize an eligible exact-revision plan within the remaining budget, then execute and publish real evidence. Head/base/plan changes require fresh eligibility and binding, but do not reset the PR's spending allowance. Duplicate events, workflow reruns, label removal/re-addition and publication retries must not duplicate paid work.
  • Account against the repository + PR number: actual charges plus outstanding/conservatively unresolved reservations across all revisions and retries must never exceed USD 200. Reserve an enforceable worst-case amount before calls, including output-token limits and the configured provider's applicable rates; concurrency must not overspend. Unknown costs, unavailable accounting, insufficient remaining budget or unsafe bounds stop execution with a visible reason. An estimate alone is not a hard cap.
  • Removing the label, closing the PR or exhausting the budget prevents new calls. Re-check authorization before subsequent calls; already in-flight charges cannot be undone. Re-enabling does not replenish the allowance. Additional budget or a materially different provider requires explicit owner authorization.
  • Show the label/eligibility state, actual execution state, spent/reserved/remaining USD and blocked reason in the existing PR report. An unsupported, stale or unexecuted plan stays “insufficient evidence,” not PASS.

Acceptance

  • Offline controls cover fix, regression, unchanged-correct, unchanged-incorrect, missing pairs, grader/runner errors, uncovered scope, no selected cases, stale revisions and expired/missing artifacts. No aggregate score hides a regression or evidence gap.
  • A controlled degraded candidate produces actionable negative guidance; a prepared-only run cannot support a positive recommendation. Publication retry makes no new model calls.
  • Before relying on results, reconcile relevant fix: run state and comparison verdicts can report support the artifacts do not establish #179 plan/verdict-integrity items with the then-current code, tests and merged fixes. Record remaining blockers; do not assume every historical defect remains present or require unrelated lifecycle work merely because fix: run state and comparison verdicts can report support the artifacts do not establish #179 is open.
  • Demonstrate Phase 1 on the selected addition/update PRs with exact revision-bound reports and an explicit reviewer decision rationale. Real runs must use revision-bound authorization, either the implemented owner-enabled label policy within the USD 200 per-PR ceiling or a separately approved exact-plan execution. Until that mechanism is implemented and verified, do not treat this issue edit as a live execution switch. Negative or inconclusive results can complete the demonstration without authorizing a subject PR merge.
  • Validate label-gated execution with offline controls for unauthorized label actions, duplicate events, concurrent reservations, head changes, removal/re-addition, cancellation, unknown charges and budget exhaustion. Demonstrate an authorized real run and evidence publication without exceeding the cumulative PR cap; report any live-validation prerequisite explicitly.
  • Record the Phase 2 selection and its bounded acceptance, or explicitly defer it with reason and destination. Do not claim expanded coverage if deferred.
  • Update the existing usage guide, including obsolete readiness statements, and record revision-scoped independent review plus applicable offline/CI results.

Boundaries and ownership

One accountable issue owner coordinates implementation and independent review; no existing claim is implied. Confirm active claims before editing shared evaluator files. Preserve #91's separate metadata/content work.

The owner-approved automatic-execution scope is limited to suitable, explicitly enabled PRs and USD 200 cumulatively per PR. Publication of this requirement does not itself enable a label, dispatch a run or configure credentials. Provider/permission or protection changes, automatic promotion, merging and closing subject PRs remain outside this issue's execution authorization. No universal efficacy threshold, mandatory positive effect, or full-library evaluation is required. Close only with a scoped delivered/verified/limits summary and applicable owner acceptance.

Accepted bounded delivery and remaining limits: #215 (comment)

Activity

  1. self-assigned this
    on Oct 28, 2026
  2. Dong1017 commented on Oct 28, 2026

    @Dong1017
    CollaboratorAuthor

    Claiming implementation coordination for this issue as Dong1017, starting from main 0b19c5c3b649ebac939ffecae5583d3a22f598e7.

    First slice: reconcile relevant #179 evidence-integrity behavior, implement revision-bound review guidance, and design the label-gated execution path with a cumulative USD 200 per-PR budget using the existing plan/authorize/execute/publish pipeline. I will keep the work in focused changes with independent review, and avoid duplicating #91's coverage work.

    flowchart LR
     A[Existing pipeline and integrity audit] --> B[Review guidance]
     A --> C[Label authorization and PR budget accounting]
     B --> D[Offline controls and independent review]
     C --> D
     D --> E[Explicitly enabled real-PR validation]
    
    Loading

    No paid run or credential/settings change is started by this claim. Runtime activation will follow verified controls and the agreed eligible-PR scope. I will report concrete blockers and remaining acceptance items here rather than treating PLANNED as evaluated.

  3. added
    area:knowledgeKnowledge line: packs, skill library, knowledge admission
    kind:taskBounded delivery task with a delegated slice
    on Oct 28, 2026
  4. Dong1017 commented on Oct 28, 2026

    @Dong1017
    CollaboratorAuthor

    Deployment preflight: #216 is merged; #223 contains the independently reviewed label/controller/ledger/request-guard implementation. The dedicated skill-eval:execute label exists but is not applied to a paid target.

    Historical retained-workflow audit for the owner-approved #159/#160 validation found three paid runs, including a failed #160 attempt. Conservative genesis charges, rounded upward:

    These are upper bounds, not invoices. Scope is retained official repository workflow history; no outside/deleted runs are known or claimed excluded universally. Audit JSON SHA256: 0cf72dd857d642ab9f6e810c100bb8d661cbb4c28568455dc49eca6c58cea501.

    Fresh public pricing observed 2026-10-28 10:53:48 UTC at https://yibuapi.com/pricing and its pricing API (version a42d372ccf0b5dd13ecf71203521f9d2): default-group USD8/40/0.4/10 per million input/output/cache-read/cache-write tokens. Token-only tariff implies no separate request fee; this is not an account invoice or contractual quote-validity guarantee.

    flowchart LR
     A[Audited legacy obligations] --> B[Pinned per-PR genesis]
     B --> C[Reserve new plan within cumulative USD200]
     C --> D[Label-authorized bounded execution]
    
    Loading

    No new provider calls have occurred. Next: complete #223 CI, retain reviewed genesis/configuration, then enable only the approved PRs under their cumulative limits.

  5. Dong1017 commented on Oct 28, 2026

    @Dong1017
    CollaboratorAuthor

    Live Phase 1 completed with implementation fbc9d580d8e536480cd33e965e0c2af0085dcdc4 (#229), comparison base 51fa1d31cc4a805490ddfe8055a36c81d70c4f08:

    • skill: add fixed-workload claim audit procedure #159 head 671fef22e00b0ffbe8464f39d2515f34a05419c1, plan 309ea989d57c55b2adc8c80a98d6bab78ff6c48775fd37d848705af154119e3b: run 40700386355, 5 provider requests, both trials completed. Frozen grader reports regression: baseline PASS; candidate answers INCONCLUSIVE at exactly 20%. Arithmetic is correct; candidate refuses to infer equality semantics from min_reduction_pct: 20, while the grader treats the minimum as inclusive. Keep this result; adjudicate contract clarity versus overly cautious guidance before subject-PR acceptance. No silent exam rewrite.
    • skill: clarify fixed-workload report aggregation #160 head 8a765e774f4a40023114a902de83915f05a86426, plan 2e89073474c49b40884204c2f82e0d4a7cc61cdc35149bd3bb1284a702e89cc1: run 40700395804, 8 requests, both trials PASS: unchanged_correct, neutral evidence rather than measured benefit. Source/integration review and uncovered scope remain separate.

    Both plan/execute/publish chains succeeded. Six artifact ZIP hashes and original-plan/report/comment bindings were checked; four trials have complete usage records and no evaluation error. This is 13 actual API requests, not fixed-output-only validation.

    Budget readback retains $122.04 unresolved reservation per PR, plus historical upper bounds $0.68/$63.53; remaining $77.28/$14.43. Provider costs are null, so these are conservative obligations, not invoices. Execution labels have now been removed; no additional paid run is requested.

    flowchart LR
     A[Authorized label] --> B[Permanent PR budget reservation]
     B --> C[13 provider requests / 4 completed trials]
     C --> D[Trusted retained-result grading]
     D --> E[159: threshold-semantic regression]
     D --> F[160: unchanged correct]
    
    Loading

    Phase 2 is explicitly deferred to #91's coverage follow-up: no additional pending family with independent expected outcomes, boundary cases and a suitable trusted adapter has been selected. Do not force unrelated skills through this audit grader; no expanded coverage is claimed.

    Remaining closure work: refresh obsolete usage-guide readiness statements and fix a discovered reporting edge: a free label-removal/duplicate event must not overwrite valid same-plan retained results with PLANNED. Preserve authenticated evidence and reject stale-plan reuse. #179's bounded integrity work is already accepted/closed; its recorded provenance/Windows limits remain.

  6. Dong1017 commented on Oct 28, 2026

    @Dong1017
    CollaboratorAuthor

    Scoped acceptance / closure

    Delivered by #216 (decision-useful guidance), #223 (label/controller/cumulative budget), #229 (actual run identity corrections), and #230 (same-plan evidence retention + current documentation). Final main: 1cd516baf433fa4bec464cc7726086cf0b846d31.

    Acceptance evidence:

    flowchart LR
     A[Opt-in and cumulative budget] --> B[Real paired execution]
     B --> C[Authenticated retained evidence]
     C --> D[Actionable review guidance]
     D --> E[Independent subject-PR decision]
    
    Loading

    Scope limits: labels are removed; no additional paid run. Exact bills are unavailable, so $122.04 per PR remains conservatively reserved, with $0.68/$63.53 historical obligations and $77.28/$14.43 remaining. Those reservations are not refunded or reset. Retention after unpaid events is offline-verified; deploying #230 changes plan identity, so earlier real results remain historical and cannot be rebound. This closes the bounded software/advisory delivery, not subject PRs, knowledge maturity, whole-library efficacy or a required universal merge gate. The new reports inform review; they do not automatically approve or merge skills.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:knowledgeKnowledge line: packs, skill library, knowledge admissionkind:taskBounded delivery task with a delegated slice

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions