[Dong1017]

Refs #355

Reader-only migration prerequisite

This separates the backward-compatible SkillEval reader and its tests from prototype #359, so the trusted implementation can understand the shared manifest before any format migration. The design decision on #355 accepts the shared source-snapshot manifest and puts this reader first in the rollout.

Only two files change: tools/skill_eval_pr.py and tests/test_skill_eval_planned_guidance.py (197 additions, 18 deletions). The base is 01c73521ced6ca760cdfa42c9b937fe985a20d64; the current head is 69ce133f1396ede0ddab7b9e2834709b406f17ad. Main has not modified either file since the base.

This PR adds the reader only. It does not migrate stored projections or complete #355; the #359 migration and its protected-path authorship follow separately.

Behavior and boundaries

• Read the fixed manifest path from bounded, regular raw Git blobs. Validate exact schema/fields, duplicate keys, repository identity, full source revision, registry path and SHA-256.
• Retain source ancestry and complete canonical-library equality checks. Missing, malformed, nonancestor, unsafe-mode or mismatched inputs remain unavailable.
• Preserve the legacy README reader when the manifest is absent. An invalid manifest never falls back to an otherwise valid README.
• No new imports or runtime dependencies. No generator, catalog, canonical library, stored projection, workflow, case mapping, budget, credentials or policy changes.
• The privileged workflow continues using trusted main and treating PR objects only as data. This does not run candidate Python, a model, or paid evaluation.

Fixture fix at 69ce133f

This addresses the Windows checkout finding. Only the test file changes (28 additions, 8 deletions); tools/skill_eval_pr.py is unchanged from c9b1608d.

• Manifest fixtures hash the committed registry blob (git cat-file blob <pin>:<path>) instead of working-tree bytes, which core.autocrlf=true converts to CRLF.
• The mode mutation and the executable-registry case stage 100755 with git update-index --chmod=+x, since chmod is ignored where core.fileMode is false. Both tests assert the committed mode (100755, or 120000 for the symlink case), and the unsafe-registry case uses a checksum that matches the committed blob, so each negative case is rejected only for its unsafe mode.

On Windows with Python 3.13 and imports bound to the checkout:

Run	Original head c9b1608d	This head
Manifest tests, core.autocrlf=true	valid-manifest case fails; executable cases pass only through the checksum mismatch	only the two symlink cases fail
Manifest tests, autocrlf off	mode and executable cases accept a manifest they should reject	only the two symlink cases fail
Whole guidance file, core.autocrlf=true	not run	47 passed, 4 failed

The two symlink failures are WinError 1314: this host lacks the privilege to create symlinks. The remaining whole-file failures are in the older README-pin tests, which use neither changed helper. The same README-pin failures reproduce on the original head c9b1608d and predate this PR, including the README chmod case noted as non-blocking in the review. Ruff and git diff --check pass. Hosted Linux CI is the authoritative run for this head.

Earlier verification (c9b1608d)

The original author reported the complete pytest suite, full Ruff and 51 focused guidance tests passing on an isolated Python 3.12 tree, and a read-only comparison against the #359 head in which the old main planner returns unavailable while this reader returns ready with 0 comparisons / 0 calls. Hosted Ready CI and knowledge-model-policy passed for that head.

Authorship and admission

The original reader and tests were authored by dot (projection planner) and dot (index implementation), with actual model/effort unknown; their commit keeps those trailers. The fixture fix commit was authored by Claude Code / claude-opus-5-5 at effort xhigh.

Both paths are outside protected_paths and policy_paths, so knowledge-model-policy reports this PR as Not applicable. This classification does not exempt any protected content or cure #359's authorship admission. No policy change or exception is requested.

Staged rollout

1. Review this reader through normal repository checks and approval requirements.
2. After it merges, start a new main workflow_dispatch with action=plan for prototype: reduce knowledge projection and declaration churn #359. An old-run retry stays tied to its previous trusted workflow revision.
3. Reconcile prototype: reduce knowledge projection and declaration churn #359 with that main without force-pushing and refresh its revision-bound evidence.

No Issue closure is requested.

[github-actions]

Skill evaluation

PLANNED: not executed; there are no task outcomes yet.
No approval, model efficacy or current-head test result is implied.

• Next: resolve check applicability at head 69ce133f1396ede0ddab7b9e2834709b406f17ad.
• Task selection: no supported automatic comparison; choose a relevant source/content check and disposition uncovered claims.
• Selected comparisons: 0 · Estimated task calls: 0. Uncovered/unsupported changes: 2; Canonical library paths: 0; Generated projection paths: 0; Other paths: 2. These are paths, not independent claims.
• Return: command, input/source pin, exit status, outputs/failures and untested claims for independent verified/refuted/unverified disposition.

Author verification steps (nonpaid; not yet run)

1. Bind the checkout. Run git rev-parse HEAD; it must equal 69ce133f1396ede0ddab7b9e2834709b406f17ad. If it differs, obtain a current plan; do not reuse old results.
2. Choose an applicable check. Match each changed behavior to the task below or a source/content check; a path match alone does not establish relevance.
   No supported automatic comparison is selected. Review the canonical changes and explain which claims need evidence, are unaffected, or remain unverified; do not treat generated records as separate behavioral claims.
   Uncovered scope by location: Canonical library paths: 0; Generated projection paths: 0; Other paths: 2. Path counts are not claim counts; no metadata-only conclusion is inferred.
3. Return evidence for independent review: head, check/task/source pin, command, environment, exit status, outputs/failures and untested claims. Label scripted controls. Request each claim's disposition: verified, refuted or unverified; this plan proves neither model provenance nor benefit.
   Check recipes and evidence limits

Evidence and logs · Retention: 30 days.
Retained artifacts are verifiable only before Unix time 1796801580 while the linked artifact is available. Expired evidence is unverified.

Plan bindings, budget, frozen exam and advisory details

Advisory: obtain missing evidence

Retained evidence availability: planned / unexecuted. No approval or merge authorization is issued.
Applicable only while these revisions and inputs match the current PR.
Paid execution requires trusted label authorization and cumulative PR reservation. Unknown accounting blocks new paid calls; planning is not execution.
Pipeline evidence only; offline fixed outputs do not establish skill efficacy.
Unchanged-correct is valid neutral evidence, not incremental benefit.
Content/source review and authorship admission remain separate.
Trusted regrading checks retained answers, not model/hardware provenance or a hostile executor; this is not an efficacy certification.

• Retained evidence is planned / unexecuted.
• No supported comparison cases were selected.
• Uncovered/unsupported scope needs a case or explicit reviewer disposition.
  Plan SHA-256: dc734c1f15bbeb5ec4157a90a5f32ec0bd0fe1184b27851a94a02e940b94fb0c
  Base: 01c73521ced6ca760cdfa42c9b937fe985a20d64 · Head: 69ce133f1396ede0ddab7b9e2834709b406f17ad
  Trusted implementation: 952e510760eec528e338d474ea08acc737829793

Model/settings: {"effort":"not_applicable","name":"claude-opus-5-5"}
Budget: {"comparisons":0,"estimated_calls":0,"limits_per_comparison":{"campaign_seconds":600,"max_task_calls":2,"repeats":1,"timeout_seconds":180,"token_stop_new_trials":200000},"provider_config_sha256":"b28d80f8194fbe0f5a9f01515efb24b218979fd45f702776560da276f4b30d04","reservation":null}

Skill	Comparison	Cases	Estimated calls

Automatic execution / cumulative PR budget:

{"reason":"Trusted configuration or pricing missing, invalid or expired","remaining_usd":"unknown","reserved_or_unresolved_usd":"unknown","spent_or_legacy_usd":"unknown","state":"blocked"}

Provider/settings: {"client":{"api_version":"2023-06-01","endpoint":"https://yibuapi.com/v1/messages","kind":"anthropic-messages-full-context-v1","max_output_tokens":4096,"max_read_bytes":32768,"max_request_bytes":262144,"max_requests":1,"max_tool_calls":12,"provider":"yibuapi"},"limits":{"token_stop_new_trials":200000},"model":{"effort":"not_applicable","name":"claude-opus-5-5"},"pricing":{"currency":"USD","expires_at":1793271228,"group":"default","max_input_tokens":1000000,"model":"claude-opus-5-5","rate_evidence":"https://github.com/InferMatrixLab/InferRSI/issues/215#issuecomment-6467531341","rates":{"cache_read":"0.4","cache_write":"10","input":"8","output":"40"},"request_fee":"0","unit":"per_million_tokens"},"spend_limit":{"amount":"200","currency":"USD"}}

Uncovered / unsupported paths (2)

• uncovered: "tests/test_skill_eval_planned_guidance.py"
• uncovered: "tools/skill_eval_pr.py"

[amy-why-3459]

Review (independent; COMMENT only — not an approval)

Reviewed range: 01c73521ced6ca760cdfa42c9b937fe985a20d64 .. c9b1608d0cf59f53d459fccb99acb3292abdc536 (full review, new PR; 2 files, +177/−18). Refs #355, split from #359.

What I checked

• tools/skill_eval_pr.py L77–158 (projection_blob, projection_object, projection_check): manifest presence is authoritative (blob_exists also matches a tree, which then fails in projection_blob → unavailable, so the directory case cannot fall back to README); exact key sets, type(...) is int for schema_version, duplicate-key rejection via object_pairs_hook, full 40-hex pin, repository identity, fixed registry path, 64-hex digest; regular 100644 blob + size bound; RecursionError added to the fail-closed set. Ancestry, cat-file -t commit, library tree mode and pin..head library diff are retained unchanged for both the manifest and legacy README paths.
• tests/test_skill_eval_planned_guidance.py L297–410: the 26 manifest mutations, the uncommitted-worktree negative, and symlink/executable registry at both revisions cover the reader's fail-closed branches.
• Hosted checks on this head: full-check / installed-package (3.10, 3.11), lint-and-smoke, pr-references, declarations, skill-eval plan all success.

I found no code defect in the reader itself. I'm not approving yet because of the scope questions below.

Open questions (blocking Approve for now)

1. Design acceptance on trim & improve: reduce knowledge contribution cost and generated-file churn #355. trim & improve: reduce knowledge contribution cost and generated-file churn #355 asked for the trade-off write-up to be accepted before landing a preferred design. This PR only adds a reader, but it fixes the v1 manifest schema (schema_version/repository/source_revision/registry{path,sha256}) and its authority rule (manifest present ⇒ README ignored) into trusted main. Could a maintainer confirm on trim & improve: reduce knowledge contribution cost and generated-file churn #355 that landing this reader ahead of accepting the shared-manifest representation is in scope? If the representation later changes, this reader would need a follow-up change rather than a no-op.
2. Draft status. The PR is Draft and the description asks to keep it Draft. Please mark it ready for review when you want a formal verdict; I'll re-review the then-current head.

Optional (non-blocking)

• tools/skill_eval_pr.py L151–155: given the pin..head LIBRARY_PATH diff must already be empty, the registry digest check at both pin and head reads the same blob twice. Fine as defense-in-depth (and the mode check at the pin revision is tested), just noting it's intentional.

No approval implied.

[Dong1017]

Head / conclusion: c9b1608d0cf59f53d459fccb99acb3292abdc536, base 01c73521ced6ca760cdfa42c9b937fe985a20d64. No blocking code finding in the two-file reader-compatibility slice. This is a COMMENT-only technical review through Dong1017, the PR author's account, not an eligible independent approval.

Verification

• Confirmed the complete two-file diff and identical Git blobs in the published head, reviewed local checkout and #359 d1d64da1. No generator, catalog, canonical knowledge, projection, mapping, workflow or policy change is included.
• Traced manifest presence, exact schema/type/duplicate-key validation, bounded regular-blob reads, fixed registry path/hash, commit ancestry and complete canonical-library equality. Invalid manifests remain unavailable without falling back to README; absent manifests retain the legacy reader. Planning adds a proposed check, not candidate execution.
• Independently reran all 51 guidance tests on the exact head with Python 3.12.14, plus focused Ruff and whitespace checks; the checkout remained clean. Tests cover malformed metadata, unsafe modes, missing/nonancestor commits, mismatched source bytes and legacy behavior.
• Hosted Ready CI completed successfully for both Python 3.10/3.11 full-check and installed-package jobs, plus PR references. Lint/smoke passed; the Draft KV job was skipped. Knowledge-policy logs evaluate this exact head under 01c73521 as Not applicable, with no errors.

Limits
No full-suite or installed-wheel rerun was performed locally in this review, and no model/device evaluation ran. The hosted SkillEval plan has zero comparisons/calls; it does not test this candidate reader against #359 because it uses trusted main. A fresh plan after any authorized merge remains the rollout check. This result does not accept #359's representation/squash trade-off, resolve its unknown-authorship admission, or complete #355. The PR remains Draft; no approval or merge is implied.

[Dong1017]

@zengchuang-hw @amy-why-3459, this PR is now ready for review at c9b1608d0cf59f53d459fccb99acb3292abdc536. Could you please review it?

• The scope question from the earlier review is answered on trim & improve: reduce knowledge contribution cost and generated-file churn #355: the design decision accepts the shared source-snapshot manifest and puts this reader first in the rollout.
• The head is unchanged. Main 3a67f2456156f1071d4821e172a5bb55393f3f08 has not modified tools/skill_eval_pr.py or tests/test_skill_eval_planned_guidance.py since the base 01c73521, and the hosted checks on this head passed. knowledge-model-policy reports this PR as not applicable, since both paths are outside the protected scope.

This PR does not complete #355. The #359 migration follows separately after this lands.

[zengchuang-hw]

Review range c9b1608d → c9b1608d (same-head follow-up to Amy’s earlier review). I reviewed the two-file diff from 01c73521 through current head c9b1608d0cf59f53d459fccb99acb3292abdc536 for main. Conclusion The manifest reader is within scope. A newly added guidance test fails under Git checkout line-ending conversion, so this is a non-voting review on the Ready request.

本轮已解决 · Resolved

• The owner decision accepts the shared source-snapshot manifest and the reader-first rollout. The PR is Ready and the two changed paths have no overlap with main’s intervening changes through c38bb4e. Issue #355 decision
• The reader verifies the raw registry blob at the declared revision, checks ancestry and canonical-library equality, rejects invalid authoritative manifests, and preserves the legacy README path when the manifest is absent. Reader diff

批准前必做 · Before I can approve

• Dong1017, derive the synthetic manifest checksum from the committed registry blob at the pinned revision. The fixture currently hashes canonical.read_bytes(). With core.autocrlf=true, Windows writes CRLF to the fixture, Git stores LF, and the valid-manifest test returns unavailable. Use raw bytes from git cat-file blob <pin>:<registry-path> in the fixture. The same mismatch can mask the executable manifest and registry cases. Stage mode 100755 explicitly with git update-index --chmod=+x after adding the paths. The item closes when the valid case passes with core.autocrlf=true, both executable cases reject confirmed 100755 blobs with a matching registry checksum, and Ready CI and knowledge-model-policy succeed on the updated head.

非阻塞问题或建议 · Non-blocking

• The Windows run also hit the older README fixture’s chmod case. That case predates this PR. Baseline test

Verification Ruff and whitespace checks passed. The exact-head guidance file on Windows yielded 49 passed and the two failures described above. Disabling checkout conversion for the targeted run made the valid-manifest case pass and exposed the two executable-mode fixture failures. A separate synthetic probe with core.autocrlf=true verified that the raw committed checksum lets the reader accept the manifest despite an uncommitted replacement. GitHub’s Ready workflow and knowledge-model-policy passed for this head, with policy reporting not applicable. The hosted SkillEval plan used trusted main and ran zero comparisons or model calls. The #359 rollout remains separate.

Codex performed this review under direct delegation from zengchuang-hw, using that connected account. The authorization covers inspection of the exact head, reporting findings and approving eligible verified heads.

[cursor]

LGTM

PR review verdict — PASS
Reviewed base 01c7352 and head 69ce133.
Full diff against main. Prior reviewed head: c9b1608.

Earlier finding — resolved

• Checkout line-ending conversion. On c9b1608, zengchuang-hw reported a new guidance test failed when Git checkout conversion (core.autocrlf) changed working-tree bytes. The current tests hash git cat-file blob <rev>:<path> and stage executable bits with git update-index --chmod=+x, then assert recorded tree modes 100755 and 120000. The reader uses the same raw ls-tree / cat-file bytes, so the declared registry sha256 is the stored blob digest.

Verified

• tools/skill_eval_pr.py projection_check: a present _provenance.json is authoritative. The schema is exactly schema_version, repository, source_revision, and registry; schema_version must be the integer 1; duplicate keys fail closed; the repository and registry path are fixed; sha256 is 64 lowercase hex digits. The pin must be a full SHA and an ancestor commit. LIBRARY_PATH must be an unchanged tree. Both revisions' skill-index.json objects must be mode 100644 blobs whose sha256 matches the manifest (manifest cap 64KiB, registry cap 16MiB). Symlinks, executables, directories, and oversized inputs stay unavailable. The legacy README pin is used when the manifest path is absent. The result is a proposed check only.
• tests/test_skill_eval_planned_guidance.py: authoritative committed pin with a dirty worktree, the invalid-manifest matrix (unsafe modes forced through update-index), and unsafe registry modes at the pinned revision and HEAD.

Verification limit

• This session did not re-run the guidance tests, Ruff, or CI. The prior comment text available here ends in the zengchuang-hw “Before I can approve” section; the visible open item was the checkout line-ending failure above.

[amy-why-3459]

LGTM

PR review verdict — PASS
Reviewed base 01c73521ced6ca760cdfa42c9b937fe985a20d64 and head 69ce133f1396ede0ddab7b9e2834709b406f17ad; incremental range since my last review c9b1608d0cf59f53d459fccb99acb3292abdc536...69ce133f1396ede0ddab7b9e2834709b406f17ad (one test-only commit).

Earlier items resolved:

1. Design acceptance: the #355 owner decision accepts the shared source-snapshot manifest and puts this reader first in the rollout, which answers my scope question.
2. Draft: the PR is now ready for review.
3. zengchuang-hw's line-ending finding: tests/test_skill_eval_planned_guidance.py now hashes the stored blob via committed_blob() (git cat-file blob <rev>:<path>) instead of working-tree bytes, stages executable modes with git update-index --chmod=+x, and asserts the committed tree mode (100755 / 120000) so each negative case fails for its intended reason. In the unsafe-registry cases the checksum matches the committed blob and source_revision is the commit carrying that mode, so the mode is the only reason to reject.

Verified:

• tools/skill_eval_pr.py is unchanged since my previous reader review at c9b1608d; this commit touches only the test file.
• Hosted checks on this head are green: full-check (3.10/3.11), installed-package (3.10/3.11), lint-and-smoke, declarations, pr-references, skill-eval plan; GitHub reports the PR mergeable (clean).

Not verified by me: a local run with core.autocrlf=true / core.fileMode=false (hosted CI is Linux), and any executed SkillEval comparison (zero selected).

[Dong1017]

@zengchuang-hw, the fix for your Windows checkout finding is at 69ce133f1396ede0ddab7b9e2834709b406f17ad. Could you please re-review? amy-why-3459 has approved this head.

• Only the test file changed. The manifest fixtures hash git cat-file blob <pin>:<path>, the mode and executable-registry cases stage 100755 with git update-index --chmod=+x, and both tests assert the committed mode. The unsafe-registry checksum now matches the committed blob, so the mode is the only reason to reject.
• On Windows with core.autocrlf=true, the manifest tests now fail only in the two symlink cases (WinError 1314, no symlink privilege on this host). With autocrlf off, the mode and executable cases now reject as intended. The older README-pin failures reproduce unchanged on c9b1608d.
• Hosted Ready CI passed on this head. knowledge-model-policy reports Not applicable.

[zengchuang-hw]

LGTM

PR review verdict — PASS

Incremental review c9b1608 → 69ce133, current head 69ce133f1396ede0ddab7b9e2834709b406f17ad targeting main. The previous review finding is resolved. This addresses the re-review request.

本轮已解决 · Resolved

• The fixture hashes committed registry bytes with git cat-file blob, so checkout line-ending conversion cannot invalidate the manifest. Both executable cases now stage mode 100755 explicitly and verify the committed mode. The unsafe-registry fixture hashes the actual blob at the pinned revision. Test-only delta
• The reader is byte-identical to the prior reviewed head. The reader-first change remains in the accepted #355 scope. The current main changes neither touched file relative to the PR base.

批准前必做 · Before I can approve

No outstanding items.

非阻塞问题或建议 · Non-blocking

The author reported missing symlink privileges on their Windows host. The manifest selection on my Windows environment passed both symlink cases. The older README test compatibility remains separate from this test-only fix.

I ran the 29 manifest cases with core.autocrlf=true and core.filemode=false, and ran Ruff and the diff whitespace check. All passed. The exact-head Ready workflow and knowledge-model-policy passed. The policy reports not applicable. I did not rerun the entire suite locally or execute any model comparisons.

Codex performed this review under direct delegation from zengchuang-hw, using that connected GitHub account. The delegation covers independent inspection and feedback on this PR and a formal approval bound to the verified head when the applicable checks pass.