Skip to content

chore: retire the synthetic CPU KV slice tooling - #357

Open
Dong1017 wants to merge 1 commit into
InferMatrixLab:mainfrom
Dong1017:chore/retire-synthetic-kv-slice
Open

Dong1017 wants to merge 1 commit into
InferMatrixLab:mainfrom
Dong1017:chore/retire-synthetic-kv-slice

Conversation

@Dong1017

@Dong1017 Dong1017 commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Governing Issue and delivery slice

Refs #248

Removes the tooling of the bounded synthetic CPU KV experiment. All retained evidence stays in place and unmodified. This is a cleanup of code that no longer has a path to run; it decides nothing about #248's acceptance, which remains open with neither completion criterion met.

Why this is removable

The slice had its own consumers only. Nothing in the general harness, the SkillEval modules, the runner or the evaluator imported it: git grep -E '\bkv_|kv\.' across src/inferrsi/harness/skill_eval*.py, evaluation.py, runner.py, improvement.py, reporting.py and tools/skill_eval_*.py returns nothing. The dependency ran one way only — from this slice onto shared parts (skill_eval_engineering.sandbox_command, skill_eval_budget, harness.runner.run_experiment, infra.artifacts, model.anthropic_agent, runspec.RunSpec). Removing it cannot reduce the general SkillEval capability.

Its only non-self consumers were the kv-task-qualification job in the ready lane — self-described there as "not a branch-protection or efficacy gate" — and four rows in docs/operations.md.

What changed

Removed Count
tools/kv_*.py, tools/qualify_kv_*.py, tools/kv_live_authorization.json 12
src/inferrsi/harness/kv_reclamation.py, kv_edit_workbench.py 2
tests/test_kv_*.py, tests/test_qualify_kv_*.py 13
tests/fixtures/kv_reclamation/ 3
docs/kv-edit-integration.md, kv-live-launcher.md, kv-pilot-admission.md 3
.github/workflows/kv-edit-pilot.yml 1

Also: the kv-task-qualification job drops out of .github/workflows/ci-ready.yml (52 lines), and the four operator rows drop out of docs/operations.md.

What is kept, and one constraint that forced it

evidence/kv-reclamation-20261030/, kv-actor-qualification-20261030/, kv-actor-qualification-20261031/ and kv-live-pilot-20261031/ are untouched — 64 files, 2.3 MiB — along with their four entries in tools/evidence_inventory.json.

docs/kv-reclamation-qualification.md stays at its path, rewritten as a retirement record rather than deleted. evidence/kv-reclamation-20261030/README.md:59 links to it, and that README's SHA-256 is pinned in its own plan/publication-integrity.json (7c6d65e4…), so the link cannot be edited out without breaking the manifest. Deleting the page would leave a dangling link inside hash-pinned evidence. The rewritten page records what was removed, where the evidence lives, and what that evidence does and does not show.

src/inferrsi/model/anthropic_edit_agent.py is also kept. Its only current consumers are the removed code and its own test, but it is a general edit-capable model adapter rather than an experiment-specific module. Removing it would widen this change beyond cleanup; flagging it here so a maintainer can decide separately.

Checks

Local, on Python 3.13 at this head:

  • Test collection: 3557 → 3183, no collection errors. The 374-case drop is exactly the removed test files; no module fails to import, which is the decisive check for dangling references.
  • ruff check src tests tools: passed.
  • python -c "import inferrsi.harness, …skill_evaluation, …skill_eval_engineering, …runner, …evaluation": passed.
  • python -m tools.validate_evidence: exit 0, 430 hashes checked, all 21 bundles including the 4 retained KV bundles verified.
  • git diff --cached --check: clean. docs/operations.md shows exactly 4 removed lines, with no line-ending noise.

Not run locally: the full suite. Several of this repository's tests probe bubblewrap isolation and time out on Windows, so a full local run is not meaningful here. The ready lane runs it on Ubuntu; that is the full-suite evidence for this PR, and I am not claiming a local pass.

Limitations

This changes no behavior of any remaining feature. It is not an acceptance, rejection or rescoping of #248: both of that Issue's completion criteria are still unmet, its two 2026-10-31 attempts remain INCONCLUSIVE, and the retained evidence that records those failures is deliberately preserved here. Removing the tooling does not make the experiment repeatable or unrepeatable — the Issue's own freeze already forbids a third attempt.

Merge and any scope decision on #248 remain with the maintainers.

Remove the bounded synthetic KV experiment's tooling, tests, task fixtures,
operator pages and manual pilot workflow, plus its ready-lane qualification
job. The slice had its own consumers only: nothing in the general harness,
the SkillEval modules, the runner or the evaluator imported it, so the
dependency ran one way onto shared parts.

Retained evidence is untouched. docs/kv-reclamation-qualification.md stays at
its path, rewritten as a retirement record, because a hash-pinned evidence
README links to it and must not be edited.

Agent-assisted-by: Claude Code / claude-opus-5 (author)
Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-05T09:51:30.490560Z 029878b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copy link
Copy Markdown
Collaborator

Review at head 029878bbb000d82e6e6d38d895c0865a4adbf43b (full review of the PR, base e843b9d3). The removal itself looks clean: no remaining import of the removed modules in src/, all Ready checks are green at this head, and docs/operations.md loses exactly the four KV rows. I am not approving yet because of two points I would like resolved or confirmed:

  1. Retained hash-pinned evidence still points at removed paths, and the retirement page does not say where to find them. The PR keeps docs/kv-reclamation-qualification.md specifically to avoid a dangling link from pinned evidence, but the same retained bundles also reference files this PR deletes, for example evidence/kv-live-pilot-20261031/patch/README.md:4-5 ("Both arms start from the exact unchanged tests/fixtures/kv_reclamation/candidate.py pinned by ../plan/plan.json"), and the plan/*.json / results/hosted-*/qualification.json files reference tools.qualify_kv_reclamation, tools.kv_live_job and kv_edit_workbench. Since that evidence cannot be edited, please add to docs/kv-reclamation-qualification.md (around lines 20-21 / 25-33) the last revision where the removed tools and fixtures still exist (e.g. base e843b9d31cb86e1fd12d00890c89798b0bedd1fe), so every path named in the retained evidence stays resolvable via git history.
  2. Required-check confirmation for the dropped kv-task-qualification job (.github/workflows/ci-ready.yml). The job comment says it is not a branch-protection gate, but it currently reports on every open PR (e.g. it is in the green Ready set on feat: add native WorkSwarm plugin and release artifact #327). Please confirm it is not a required status check in the main ruleset; otherwise removing it would leave other PRs waiting on a check that never reports.

Non-blocking: the claims in lines 37-42 of the rewritten page (both 2026-10-31 attempts INCONCLUSIVE, the second stopping on HTTP 403, dollar figures being reserved upper bounds) are summaries of the retained evidence that I did not independently re-check; a link to the specific receipt for each would make the page self-verifying. Keeping src/inferrsi/model/anthropic_edit_agent.py for a separate maintainer decision is reasonable.

No approve or request-changes vote at this head.

@Dong1017 Dong1017 added the area:workbench CLI workbench: profile, A/B loop, quality gates, evidence reports label Nov 5, 2026 — with ChatGPT Codex Connector

@Dong1017 Dong1017 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental read-only review at head 029878bbb000d82e6e6d38d895c0865a4adbf43b against e843b9d31cb86e1fd12d00890c89798b0bedd1fe. Scope: the required-check question in the existing review, not a fresh approval or full re-review of the removed KV tooling.

  • Verified for the active ruleset: main ruleset protect-main currently declares only knowledge-model-policy under required_status_checks; kv-task-qualification is not required by that ruleset. I also read the actual .github/workflows/ci-ready.yml removal. Therefore the proposed deletion does not remove a status check required by this active ruleset.
  • Remaining verification boundary: reading the separate classic branches/main/protection endpoint returned HTTP 403 (Resource not accessible by integration), so this does not prove that no classic branch-protection setting exists. A maintainer with settings access can confirm that last point if necessary.
  • The existing request to preserve a discoverable historical revision for removed paths in retained evidence is not cleared by this check. No tests, model calls, or device experiments were run for this review.

This is scoped COMMENT feedback, not a merge/acceptance verdict.

@Dong1017

Dong1017 commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Dear committer @amy-why-3459 @zengchuang-hw, can you review this pr when you have a chance?

@zengchuang-hw zengchuang-hw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

PR review verdict — PASS

Initial independent review of 9dded7ad..029878bb (37 changed files), at head 029878bbb000d82e6e6d38d895c0865a4adbf43b. Assessed integration with current main 01c73521ced6ca760cdfa42c9b937fe985a20d64; the PR metadata still reports base e843b9d3. The merge-tree check against current main is clean. I read the earlier review and the required-check follow-up. Conclusion: no outstanding content blocker for this retirement change.

本轮已解决

  • The removed modules, commands, fixtures and qualification job form the retired bounded KV slice. Searches of remaining source, tools, tests and workflows found no active consumer of the deleted modules. The general editing adapter remains, and the four operator rows are removed without unrelated changes.
  • All 64 files in the four retained KV evidence directories, and the evidence inventory, have unchanged Git blobs. The retirement page preserves the pinned README's documentation target. Historical source identity is already retained in the frozen plan and actor receipt, so the deletion does not erase their source provenance.
  • The dropped kv-task-qualification job is not required: the active main ruleset requires only knowledge-model-policy. A successful main branch metadata read also reports classic protection enabled=false, enforcement off and empty required-check lists. This supplies evidence beyond the earlier separate protection-endpoint 403.

批准前必做(Before I can approve)

None for this reviewed change. Issue #248 remains unfinished; this cleanup does not accept its experiment, change its frozen verdicts or authorize another attempt.

非阻塞问题或建议

Adding a direct last-available source tree link to the retirement page would make old paths easier to find. Historical analysis should still use the original revision recorded in each receipt. Links to the particular live-attempt receipts would also improve navigation. These are documentation discoverability improvements, rather than missing source provenance or a broken active operator.

Checks and limits

Independent local verification at the exact head, on Windows / CPython 3.12.1 / pytest 9.1.1:

  • 25 tests passed across test_evidence_inventory.py, test_frozen_bytes.py and test_module_boundaries.py.
  • 3,183 tests collected with no collection errors.
  • python -m tools.validate_evidence: exit 0, 430 hashes / 21 bundles checked, no errors; pre-existing external/unavailable evidence retains its stated limits.
  • Ruff 0.15.15 (src tests tools) and diff whitespace checks passed.

Current-head Ready CI passed both Python 3.10/3.11 full-check and installed-package jobs, plus PR references. The required policy result explicitly evaluates this head as successful / Not applicable. Main's intervening label/report changes do not touch the deleted KV slice or knowledge-policy scope.

I did not locally rerun the full suite or installed-wheel matrix, or execute hardware, provider or model experiments. The clean current-main merge tree was inspected, not separately run through a full test matrix. This is a PR content review, not a campaign PASS.

Direct delegation: zengchuang-hw, the account holder, authorized Codex in this chat to independently review other authors' InferMatrixLab/InferRSI PRs and submit formal Approve when content blockers are absent and applicable verification passes. Executor: Codex, with separate technical and scope/evidence cross-checks; acting GitHub account: zengchuang-hw (repository write access or higher). This account and reviewing task did not author, materially contribute to, or parent this delivery slice. Authorized action here: revision-bound APPROVE; scope: this PR's retirement diff and its verification.

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

Labels

area:workbench CLI workbench: profile, A/B loop, quality gates, evidence reports

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants