Skip to content

chore(benchmarks): skill-doctor evaluation PoC evidence - #8037

Merged
usirin merged 4 commits into
mainfrom
skill-doctor-evidence
Sep 20, 2026
Merged

usirin merged 4 commits into
mainfrom
skill-doctor-evidence

Conversation

@creosB

@creosB creosB commented Sep 5, 2026 •

Copy link
Copy Markdown
Member

Part of #8035

Evidence PR for #8035 — the tracking issue owns the five production-import decisions; this PR makes the experiment's evidence reviewable. It carries no production import: the byte-exact skill candidate lands in a separate PR after those decisions.

What's here

  • benchmarks/README.md — folder map, verification results (tests + end-to-end run), upstream pin/re-sync history, privacy posture
  • benchmarks/warp-skill-doctor-import-poc/REPORT.md — full experiment record: method, scores (B+ / 0.88, directional on 2 sessions), findings, portability notes, "revise before import" verdict
  • benchmarks/warp-skill-doctor-import-poc/ADAPTATION.md — deviation ledger: what is upstream's, what is ours, why
  • benchmarks/warp-skill-doctor-import-poc/ISSUE_DRAFT.md — kept source of skill-doctor fabrika import: five blockers before production #8035's posted body (marked FILED)
  • benchmarks/warp-skill-doctor-import-poc/results/opencode_adapter.py — the opencode→Claude-Code translation shim (upstream has no opencode collector; blocker 1's working reference). Machine-path-free: derives its scope from cwd.
  • A folder-local .gitignore keeping results/ artifacts machine-local

Deliberately not in this PR

Real session transcripts, census inventories, rendered report.html, and the import candidate (claude-plugins/fabrika/skills/skill-doctor/). The first three stay local under the ignored results/; the last waits on #8035.

Verification (2026-09-06, upstream pin b811c24365ae)

  • Upstream test suites: 25/25 (14 collect + 11 render), PYTHONUTF8=1
  • End-to-end re-run: adapter → unmodified upstream collector (2 sessions sampled, 27 skills) → rubric judging → aggregation → render_report.py → B+ (overall 0.88), identical to the original 2026-08-31 run
  • Scope note: 2 sessions, one machine — directional evidence, not a corpus measurement

Review ask

Sanity-check the docs, the blocker list, and the adapter's shape. Decisions happen in #8035; mark ready or split follow-ups from there.

Deviations

  • Said: The epic delivers a production Skill Doctor import. Did: This PR publishes only sanitized evaluation documents and the experimental adapter. Why: Evidence can be reviewed separately from the production import. Disposition: Partial delivery; the epic stays open and its children own the remaining work.
  • Said: Evaluate the upstream pipeline on real session history. Did: Used an external OpenCode-to-Claude-Code adapter and two sessions, leaving upstream scoring files unchanged. Why: Upstream lacked an OpenCode collector. Disposition: Historical execution evidence only; production collection and broader calibration remain with the epic.
  • Said: Record upstream test results. Did: Ran the suites with PYTHONUTF8=1. Why: Native Windows encoding failed on upstream UTF-8 files. Disposition: The result is explicitly environment-qualified; portable execution remains with the portability child.

@github-actions

github-actions Bot commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

No preview deploy

  • No preview deploy for this PR — its diff touches no deploy-relevant path, so no preview stack was minted and e2e is not applicable. (6eee029)
  • web — Stage pr-8037 torn down.

creosB added a commit that referenced this pull request Sep 6, 2026
…l path

The leak scan refused contract.md on a ~/.claude literal (path-policy match,
#8037/#8063 CI evidence). Describe the default by role instead; the
conformance suite stays 13/13 green locally.
@creosB
creosB marked this pull request as ready for review September 6, 2026 18:26
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

heal-ci: ROUTED — PR #8037 @ a1d3a26 → author

Scheduled stall sweep: this pull request classifies as linkage-refused, stranded 137 minute(s) at head a1d3a26fdda5d8cad8e1b52f426209fe9a1953fe.

Detection only — this run merged nothing, re-ran nothing and spawned nothing. What to do about the flag is a driver decision; /fabrika:heal-ci 8037 reads the full diagnosis.

Posted by the heal-ci-sweep workflow (#6146). The suppression key <pr>:<class>:<head> rides in the marker below, matched over the whole comment history: no further note lands here until this pull request changes class or gains a new head commit.

@usirin

usirin commented Sep 20, 2026 •

Copy link
Copy Markdown
Member

review-code: FAIL @ a1d3a26 content:42b38baae46c — error-only tool results disappear from evidence

Round 2. The prior missing-link and missing-deviations findings are resolved: scope reads part-of:8035 and deviations reads found with three entries. The epic stays open; production import, collector, portability, discovery, branding, calibration and sharing remain outside this partial evidence delivery.

FAIL: benchmarks/warp-skill-doctor-import-poc/results/opencode_adapter.py lines 96-105 reads state.output only. Lines 127-150 emit a tool_result only when that output exists. OpenCode error states instead contain state.error, so a failed tool with no output becomes a tool_use with no failure result. This silently drops evidence the report says was preserved. Official source: https://github.com/anomalyco/opencode/blob/dev/packages/sdk/js/src/gen/types.gen.ts#L266-L279 defines ToolStateError with error:string and no output field.

For this historical-evidence PR, either preserve the original adapter and explicitly document omitted error results and the resulting limits on fidelity and scores, or correct the adapter with an error-state fixture and distinguish the corrected behavior from the historical run. No production collector is required here.

Contract gap: review criteria 8035 returns absent. This epic predates the criteria section; its Goal / non-goals describes the production goal, but this PR is a partial evidence change, not the epic tail for which the skill explicitly permits that fallback. I have not invented acceptance criteria. Re-planning or supplying a conforming criteria block closes that gap. append-criterion was attempted for the fidelity finding and refused at exit 7 because there is no block.

Other standing checks: SQLite uses mode=ro; output stays local; there is no deployed behavior and no changed pre-existing test. CI settled green at this head, with all 44 checks enumerated, 40 success and four skipped. That does not prove adapter-specific error coverage. Governance is not required.

Deviation-disclosure: PASS for the three disclosed scope, adapter and environment deviations; the new fidelity limitation still needs explicit treatment in the evidence.

Verdict-written: 2026-09-20T20:23:45Z

Superseded verdict — 2026-09-20

review-code: FAIL @ a1d3a26 content:42b38baae46c — missing issue relationship and deviations

Reviewed all six changed files at a1d3a26. No governance namespace is required.

FAIL: the PR includes executable Python but has no recognized issue relationship. Its evidence-only scope should use Part of #8035, leaving the production epic open. Without that link there is no acceptance contract to grade.

FAIL: the required ## Deviations section is absent. Disclose the partial evidence delivery, external OpenCode adapter, two-session limitation, and UTF-8 environment requirement.

The adapter reads SQLite with mode=ro and limits sessions to the current checkout. It writes translated records and a manifest locally. This is an experiment helper, not a production collector or a scoring implementation. No existing test is weakened. CI at this head is green: 44 checks enumerated, 40 successful and four skipped. That does not establish adapter-specific test coverage.

deviation-disclosure: FAIL, absent.

Verdict-written: 2026-09-20T20:09:05Z

@usirin

usirin commented Sep 20, 2026 •

Copy link
Copy Markdown
Member

review-doc: FAIL @ a1d3a26 content:42b38baae46c — historical report overstates preserved failure evidence

Round 2. The original relationship and disclosure findings are fixed. Scope now reads part-of:8035; deviations contains the evidence-only scope, external adapter and two-session limit, and PYTHONUTF8 requirement.

FAIL: REPORT.md section 10 claims tool calls, patched file names, outputs and errors are all preserved. The committed adapter reads state.output but never state.error, and only emits result records when output exists. OpenCode's ToolStateError stores the failure in error:string, with no output field: https://github.com/anomalyco/opencode/blob/dev/packages/sdk/js/src/gen/types.gen.ts#L266-L279 . Error-only failed-tool results are therefore omitted. Preserve the historical record but explicitly disclose this limitation, remove the all-errors-preserved claim, and qualify what the recorded scores establish. No production collector rewrite or fresh private-session run is required to make this archival evidence honest.

Contract note: review criteria 8035 returns absent. The parent reviewer applies the pre-criteria epic Goal / non-goals fallback together with the partial-delivery rule to this evidence-only split. That is an explicit interpretation beyond the skill's named epic-tail example, not a claim that the verb served acceptance criteria. No criteria were invented; all production import, collector, portability, discovery, branding, calibration and sharing work remains open. The attempted fidelity-criterion append refused at exit 7 and changed nothing on the epic. Re-planning would close the missing-block gap. The confirmed fidelity claim above independently requires repair under the doc rubric.

Diataxis: README, REPORT, ADAPTATION and ISSUE_DRAFT are reference records. Commands in the report record the historical invocation; ISSUE_DRAFT identifies itself as the replaced filing-time body. Writing-for-agents: structure is readable and the live epic owns current work, but the unsupported fidelity claim fails claims-trace. No other doc finding from the full six-file read.

CI settled green at the reviewed head: 44 checks, 40 success and four skipped. Governance is not required. Deviation-disclosure passes for the three existing entries; the newly found limitation needs the explicit evidence correction above.

Verdict-written: 2026-09-20T20:24:27Z

Superseded verdict — 2026-09-20

review-doc: FAIL @ a1d3a26 content:42b38baae46c — missing issue relationship and deviations

Reviewed the four documentation files at a1d3a26.

FAIL: this mixed code/doc PR lacks a recognized issue relationship. Use Part of #8035 for the evidence-only delivery, leaving the production epic open. The required ## Deviations section is also absent.

The README identifies the evidence and its two-session limit. REPORT.md and ADAPTATION.md describe the historical experiment against pin 0254cbe9; the README separately records the later b811c243 rerun. ISSUE_DRAFT.md explicitly preserves the filing-time issue rather than claiming to be its current body. The production import is outside this diff.

Diataxis: README is reference, identified by its folder map and dated results. REPORT.md is an experiment reference; its commands record the invocation used rather than prescribe a present-day tutorial. ADAPTATION.md is reference, organized by file and compatibility change. ISSUE_DRAFT.md is archival reference, explicitly replaced by the live epic. No second reader task requires a split.

Writing-for-agents: historical findings and current status are identified, and the README directs readers to the live issue. No prose rewrite is required for this archival evidence. CI at this head is green with 44 checks fully enumerated. Governance is not required.

deviation-disclosure: FAIL, absent.

Verdict-written: 2026-09-20T20:09:25Z

creosB and others added 4 commits September 20, 2026 13:26
Sanitized evidence for the Warp Skill Doctor evaluation (discussion #7319,
tracking #8035): experiment docs (REPORT/ADAPTATION, the filed issue's source),
the opencode-to-Claude-Code adapter shim, and a folder README with verification
results. Real transcripts, rendered reports, and inventories stay machine-local
under a results/ ignore; the byte-exact skill import candidate lands separately
after #8035's decisions.


The revise-before-import verdict stands as filing-time history; the import now
proceeds as the approved epic (#8048 packaged import, #8049 collector, #8050-#8053
following, #8054 share-posture ADR ready-for:human). External sharing deferred,
not a v1 gate. ISSUE_DRAFT.md notes the posted body was replaced by the epic
pitch; the file stays as the filing-time source.
…path

The leak scan refused the three PoC docs on ~/.claude/projects literals
(path-policy matches, not proven leaks). Replace each with a role
description; the census findings (empty Claude sessions dir, 293 Codex
rollouts, no Warp data) are unchanged.
@usirin
usirin force-pushed the skill-doctor-evidence branch from a1d3a26 to 6eee029 Compare September 20, 2026 21:03
@usirin

usirin commented Sep 20, 2026

Copy link
Copy Markdown
Member

Repair pushed at 6eee029 after rebasing onto main.

README, REPORT, and ADAPTATION now disclose that the historical adapter omits state.error and drops error-only tool results. Both recorded scores are qualified as historical outputs over incomplete failure evidence. The adapter and measurements remain unchanged; no rerun or validation of the scores is claimed.

Prose validation passed all guards. The PR body now uses Part of #8035 and a structured deviations section. The production epic remains open. Fresh review and CI are required at this head.

— at 6eee029

github-merge-queue Bot pushed a commit that referenced this pull request Sep 20, 2026
…a-shaped (#8048) (#8063)

* feat(fabrika): land skill-doctor — byte-exact upstream vendor, fabrika-shaped (#8048)

Vendors Warp Skill Doctor (MIT, warpdotdev/common-skills @ b811c243) byte-exact
(blob-SHA verified): scorers, references, collectors, renderer, assets, both
unittest suites, LICENSE. Upstream's SKILL.md is deliberately not vendored —
SKILL.md is authored fresh as the fabrika routing surface (conventions §1/§2)
with contract.md carrying collector flags, the scoring contract, and report
artifacts by section (ADR 0296). PROVENANCE.md carries the byte-exact/editable
split and the re-copy re-sync rule. A new CI job runs both upstream suites
(#8048 §8); .gitignore covers the skill's results dir and Python bytecode.
Inert on arrival: opencode collector is #8049, corpus discovery #8051.

* test(skill-doctor): two-target proof — upstream baseline + fabrika conformance (#8048)

Proposed test adaptation awaiting maintainer acceptance. Target 1 runs both
unchanged upstream suites in an isolated temp tree against upstream's own
SKILL.md, downloaded from the pinned commit b811c243 and hash-gated before use —
the shipped fabrika SKILL.md never enters that tree. Target 2 runs a new
fabrika-authored conformance suite (13 doc-level tests, PROVENANCE-listed) over
the shipped SKILL.md/contract.md: session scoping, scoring contract with
label-to-score mappings and aggregation weights, failed-conversation threshold
and edit gate, section addressability, documented limitations, §4 literal
commands. Workflow gains shell: bash + set -euo pipefail; explicit guards for
download failure, blob mismatch, and conformance-file isolation. Replacement
verified locally; PR CI pending.

* docs(fabrika): name --claude-home's default by role, not machine-local path

The leak scan refused contract.md on a ~/.claude literal (path-policy match,
#8037/#8063 CI evidence). Describe the default by role instead; the
conformance suite stays 13/13 green locally.

* fix(skill-doctor): make the shipped corpus read the same in any repository (#8048)

Drop every ticket number, issue URL and repo name from the fabrika-authored SKILL.md, contract.md and PROVENANCE.md, and carry the byte-exact upstream files that cannot be edited as per-file exemption rows instead.

---------

Co-authored-by: usirin <umutsirin1@gmail.com>
@usirin

usirin commented Sep 20, 2026

Copy link
Copy Markdown
Member

review-code: PASS @ 6eee029 content:9a2fbb17e6e7 — historical adapter limits are explicit

Round3 reviews the original six-file evidence change plus the three-document correction at this head. No adapter or ignore-rule bytes changed during repair.

The confirmed error-fidelity finding is resolved through the historical-evidence option stated in round2. REPORT sections6,7,10,12 and Result, README Verification, and ADAPTATION B3 now disclose that the unchanged adapter omits state.error-only results. They withdraw complete-fidelity claims and state that the recorded two-session scores were not rerun or validated for error fidelity; omitted failures and their effect on scores remain unknown. This PASS approves honest archival evidence, not the adapter as a production collector.

Contract basis: review criteria8035 reports absent. The epic predates the acceptance-criteria section. This round explicitly applies its Goal / non-goals together with the partial-delivery rule to the bounded evidence split, as recorded in round2; that is an interpretation of the old-epic fallback, not a claim that the verb supplied criteria. No criteria were invented or added, and the epic remains open. Production import, collection, portability, discovery, branding, calibration and sharing are not claimed complete; replanning the epic would close its missing-block gap.

Against that bounded scope: the six files publish sanitized experiment documentation and the historical translation helper only; no production skill, private transcripts, inventories or rendered reports are included. The adapter opens SQLite read-only and writes local translated data; its known failure-evidence limitation is now explicit. The ignore rules exclude generated results except the source helper. No existing test or product behavior changes, and no product release flag is needed.

Deviation-disclosure PASS: the three entries disclose partial delivery, the external adapter/two-session experiment, and the UTF-8 environment qualification. The new fidelity limitation is explicit in the evidence itself. Silent-failure, type-design and test-gap review found no remaining undisclosed claim within the historical scope.

CI settled green at6eee029e: all45 checks enumerated,37 success and8 skipped;28 repository workflows inspected the head. No local test result substituted for CI. Governance is not required.

Verdict-written: 2026-09-20T21:15:54Z

@usirin

usirin commented Sep 20, 2026

Copy link
Copy Markdown
Member

review-doc: PASS @ 6eee029 content:9a2fbb17e6e7 — historical scores and omitted failures are qualified

Round3. The fidelity correction resolves the substantive round2 finding. REPORT section10 retracts the all-errors-preserved claim and explains state.output versus state.error. Its score section, result and collector guidance carry that limitation; README and ADAPTATION point to the same correction. The historical adapter and numerical outputs remain unchanged. Neither session run was repeated or validated for error fidelity, and the number and scoring effect of omitted failures are explicitly unknown.

Contract basis: criteria8035 reports absent on the old planned epic. This round explicitly uses its Goal / non-goals plus the partial-delivery rule for this bounded evidence split, as recorded in round2, rather than claiming the verb served acceptance criteria. The epic stays open; production import, collector, portability, discovery, branding, calibration and sharing remain incomplete. No criteria were invented or added. Replanning would close the missing-block gap.

Doc rubric: PASS. README is reference for the experiment folder and dated results. REPORT is historical experiment reference; its commands record what ran rather than prescribe a tutorial. ADAPTATION is a reference ledger; ISSUE_DRAFT identifies its archived filing-time status and points to the live epic. These remain single-mode documents. Writing-for-agents passes: the correction is dated, placed beside each affected claim, and distinguishes recorded outputs from validated conclusions. The UTF-8 qualifier and two-session scope remain attached.

Deviation-disclosure PASS: the three valid entries record evidence-only scope, source adaptation and limited sample, and environment-qualified tests. The newly discovered failure-evidence limit is now explicit throughout the reviewed evidence. No unsupported complete-fidelity claim remains.

CI settled green at6eee029e:45 checks enumerated,37 success and8 skipped;28 repository workflows inspected the head. Governance is not required. No source edits or private-session reruns were performed by this review.

Verdict-written: 2026-09-20T21:16:26Z

@usirin
usirin added this pull request to the merge queue Sep 20, 2026
Merged via the queue into main with commit 6bcc7cc Sep 20, 2026
45 checks passed
@usirin
usirin deleted the skill-doctor-evidence branch September 20, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants