Skip to content

test: add a fake prepare-to-assess handoff loop - #377

Merged
Dong1017 merged 3 commits into
mainfrom
cursor/fake-core-loop-474c
Nov 10, 2026
Merged

Dong1017 merged 3 commits into
mainfrom
cursor/fake-core-loop-474c

Conversation

@Dong1017

@Dong1017 Dong1017 commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Governing Issue and delivery slice

Refs #372
Refs #362

  • Accepted scope / delegation: After docs: record the target module layout and import gate #376, the tracker’s next suggested slice is a test-double loop for prepare, archive, bind, execute, and offline assess. This PR follows that row. It does not treat the 3+1 split as a newly accepted plan.
  • What docs: record the target module layout and import gate #376 already delivered: The directory rule and the direct-import gate are on main at 5999ced639a88d359dfd7bb1798fef9548717eb5. A clean scan is not module isolation.
  • What this PR delivers: tests/architecture_closed_loop.py is called by tests/test_architecture_closed_loop.py with injected fakes. Success stores prepared and assessment bytes only after each save. A failed prepared archive does not execute. A digest mismatch does not write a harness record. A record whose attempt id is not the bound spec is rejected before recovery, not-started, or assess, and that foreign ref is not returned as the current record. Only kind completed for the bound attempt may be assessed. not_started and recovery for the same attempt still return those reasons with their own record. A missing commit does not produce an assessment ref. An unknown controller is not marked cancelled. Exit code 0 with missing samples stays INCONCLUSIVE. Missing evidence returns INCONCLUSIVE without calling assess.
  • What it does not deliver: No production Apps package, no empty knowledge/, benchmark/, or apps/ tree, no real Knowledge read or Bundle revision, no search, no model call, no CLI or runner edit, and no new exit-code table. The first real port integration and the docs/fixture cleanup remain later slices. Issues 372 and 362 remain open.

Knowledge authorship declaration

Not applicable. This PR does not change knowledge content or Bundle revision rules. The test-local JSON digest is not inferrsi-json-v1.

{
  "schema_version": 1,
  "head_sha": "27b2e388ab6a52cea8f1568c72baf0dcfb49b666",
  "human_only": false,
  "material_ai_authors": [
    {
      "tool": "Cursor",
      "model_id": "grok-4.7",
      "effort": "unknown",
      "role": "author"
    }
  ]
}

Changed artifacts and evidence

Requirement Actual change Check / evidence and result Remaining gap
Handoff the tests actually call tests/architecture_closed_loop.py PYTHONPATH=src python3 -m unittest tests.test_architecture_closed_loop tests.test_target_module_import_gate: 13 + 4 passed Fakes are not the production cores
Attempt binding before any current-record use Mismatch returns record_mismatch with record_ref unset, before recovery, not-started, and assess test_foreign_attempt_is_not_assessed and test_foreign_attempt_is_rejected_for_recovery_and_not_started. Same-attempt recovery and not-started still pass None
Only completed is assessed Any other kind returns outcome_rejected before assess test_unknown_kind_is_not_assessed uses kind finished with a parseable record None
No production core import AST check in the test module Passed. The loop module imports only hashlib, json, and dataclasses. The check does not see importlib.import_module Dynamic imports are not scanned
Import gate still holds Existing gate test 4 passed in the same unittest command Isolation is still false
Style Touched Python files Ruff 0.17.4 passed. git diff --check clean Full pytest was not run

Checks and dependencies

Remaining work and Issue closure

  • Remaining Issue requirements / follow-up: The first real Knowledge read, one Harness CPU record, and one Benchmark offline assessment remain later slices, as does the separate fixture-path cleanup. Undecided items stay undecided: new envelope exit numbers, Collection versus search_knowledge, archive ownership versus RunStore, and the Apps directory name.
  • Goal-acceptance owner or accepted closure delegation: Not claimed.
  • Goal ready for acceptance? Not claimed

Handoff (only when an actual handoff or decision changed)

Open in Web Open in Cursor 

Exercise the handoff with injected doubles. Archive failure stops
before execution, a digest mismatch does not write a record, and an
unknown controller is not treated as cancelled.

Agent-assisted-by: Cursor Grok 4.7 (author)
Signed-off-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Dong1017 <Dong1017@users.noreply.github.com>
@cursor

cursor Bot commented Nov 9, 2026

Copy link
Copy Markdown

@zengchuang-hw @amy-why-3459 Please review head 45805721ca5645d41c94cf5da82ca3b3a1dba88b. This is the test-double handoff after #376: tests call run_loop with injected fakes. PYTHONPATH=src python3 -m unittest tests.test_architecture_closed_loop tests.test_target_module_import_gate passed (9 + 4). Ruff passed on the new Python files. It does not create an Apps package, does not read real Knowledge, and does not change CLI exit codes.

@Dong1017 Dong1017 added the area:harness Harness module: profile, A/B loop, quality gates, evidence reports label Nov 9, 2026
cursor[bot]
cursor Bot previously approved these changes Nov 9, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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
Reviewed base 5999ced and head 4580572 (full diff, first review).

Verified:

  • tests/architecture_closed_loop.py is the test-local handoff named in docs/module-layout.md. Its imports are __future__, hashlib, json, and dataclasses. The diff leaves Knowledge, Harness, Benchmark, and Bundle revision untouched, and the note keeps #371 as the Apps proposal.
  • bind_execution returns None when the re-read prepared bytes do not hash to PreparedRef.digest, and when attempt_id is missing or empty, so run_loop returns bind_rejected before harness.execute (test_bind_rejects_a_digest_mismatch_without_writing_records).
  • Prepared-archive failure returns archive_prepared_failed with executes == 0. no_record and an unparsable record return no_record with no assessment ref. not_started and recovery_required return before benchmark.assess; recovery_required leaves cancelled false.
  • Assessment-archive failure keeps record_ref and the in-memory verdict and leaves assessment_ref unset. The injected fake maps missing samples to INCONCLUSIVE. The success path stores prepared and assessment payloads under separate SHA-256 digests, and the record digest is absent from the prepared archive.
  • Digests are SHA-256 of test-local json.dumps(..., sort_keys=True, separators=(",", ":")), documented as distinct from inferrsi-json-v1. test_coordinator_does_not_import_production_cores rejects AST imports whose module name starts with inferrsi.

Not verified here: tests were not executed, and CI was not observed.

Optional follow-up (not a blocker): any other kind with a parseable record falls through to assess and reason="completed", and record_ref.attempt_id is not required to match the bound spec.

@zengchuang-hw

Copy link
Copy Markdown
Contributor

PR review verdict — DO NOT PASS

Reviewed main base 5999ced639a88d359dfd7bb1798fef9548717eb5 and head 45805721ca5645d41c94cf5da82ca3b3a1dba88b in response to the review request.

批准前必做(Before I can approve)

  • Dong1017, validate the returned RecordRef.attempt_id against the bound spec before passing evidence to benchmark.assess. I injected a Harness fake that returned a valid-looking record for attempt-other while the spec was bound to attempt-1. The loop returned completed with PASS, called assess once, and archived one assessment. That breaks the accepted attempt binding for this handoff. Close this with a negative test that rejects the foreign record before assessment, asserts zero assessor calls and no archived assessment or AssessmentRef, and keeps the existing same-attempt success and recovery cases green. Handoff code · Accepted contract B.

非阻塞问题或建议

An explicit allowlist for outcome kinds that may enter assessment would also help prevent future status additions from falling through. Earlier observation.

I ran the closed-loop and import-gate modules on the reviewed head. All 13 existing tests passed, as did Ruff and git diff --check. The separate injected case above reproduces the failure in the test double. Ready CI covers the current tests. Please verify both full-check versions on the corrected head. This review covers the fake handoff, with production port integration and hardware runs outside its scope. Current Ready CI.

Copy link
Copy Markdown
Collaborator

本次 review 范围:完整 diff(新 PR),base 5999ced639a88d359dfd7bb1798fef9548717eb5,head 45805721ca5645d41c94cf5da82ca3b3a1dba88b。当前 head 上 Ready CI 全部 success,结构上思路清楚,但暂不能通过,需要先处理以下问题:

阻塞项

  1. 执行结果没有校验 attempt 绑定:tests/architecture_closed_loop.py:129-130 拿到 outcome 后只用 _record_ref(第 97–110 行)检查字段格式,没有比较 record_ref.attempt_id == spec["attempt_id"]。Harness 返回别的 attempt 的记录时,第 147 行仍会进入 benchmark.assess 并返回 completed。这与 @zengchuang-hw 的复现一致。期望修改:绑定不一致时在 assess 之前返回一个明确的拒绝原因(例如 record_mismatch),并加负向测试,断言 assesses == 0、没有归档 assessment、assessment_ref is None。
  2. 未知 kind 会落到评估分支:第 131–146 行只排除了 no_record / recovery_required / not_started,其它任何值(包括缺失的 kind、拼错的状态或以后新增的失败状态)只要 record_ref 能解析,就会被评估并标记 completed。对这个“失败就停止交接”的回路来说,这属于默认放行。期望修改:只有 kind == "completed" 才进入 assess,其它值返回明确的拒绝原因,并补一条未知 kind 的测试。

非阻塞建议

  • 第 147 行把 outcome.get("evidence") 直接交给 assess,evidence 缺失时 fake(tests/test_architecture_closed_loop.py:35)会在 None.get 上抛 AttributeError。建议在回路里对缺失的 evidence 给出明确结果(例如 INCONCLUSIVE 或拒绝),而不是依赖 assessor 自己处理。
  • tests/test_architecture_closed_loop.py:194 的 test_missing_samples_stay_inconclusive 只断言了 verdict,建议同时断言 reason 和 assessment_ref,确认 INCONCLUSIVE 也被正确归档。
  • 第 75–86 行手动替换全局 subprocess.run/Popen,建议改用 unittest.mock.patch,更不容易在异常时泄漏;另外它拦不住 os.system 等其它入口,这点在注释里说明即可。
  • 第 196–205 行的 AST 检查只看静态 import,拦不住 importlib.import_module("inferrsi...") 这类动态导入。作为测试替身可以接受,建议在 docstring 里写明这个边界。

修好后请在新 head 上重跑两版 full-check,我会只看增量再复核。

Assess only a completed record whose attempt id matches the bound spec.
A foreign attempt or any other kind stops before assess and does not
archive an assessment.

Agent-assisted-by: Cursor Grok 4.7 (author)
Signed-off-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Dong1017 <Dong1017@users.noreply.github.com>
@cursor

cursor Bot commented Nov 10, 2026

Copy link
Copy Markdown

@zengchuang-hw @amy-why-3459 Please re-review head c933c0ae4ed81d613bc8143d23ed304149a6e7a4. Two blockers from the last review are fixed in tests/architecture_closed_loop.py. A record whose attempt_id is not the bound spec returns record_mismatch before assess, with no assessment archive (test_foreign_attempt_is_not_assessed). Only kind completed may be assessed; kind finished returns outcome_rejected and does not call assess (test_unknown_kind_is_not_assessed). PYTHONPATH=src python3 -m unittest tests.test_architecture_closed_loop tests.test_target_module_import_gate passed, 12 + 4. Ruff passed on the touched Python files.

amy-why-3459
amy-why-3459 previously approved these changes Nov 10, 2026

@amy-why-3459 amy-why-3459 left a comment

Copy link
Copy Markdown
Collaborator

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
Reviewed base 5999ced639a88d359dfd7bb1798fef9548717eb5 and head c933c0ae4ed81d613bc8143d23ed304149a6e7a4(增量 45805721ca5645d41c94cf5da82ca3b3a1dba88b..c933c0ae4ed81d613bc8143d23ed304149a6e7a4,1 个提交)。

上次意见的处理情况:

  • attempt 绑定:tests/architecture_closed_loop.py 在 assess 之前比较 record_ref.attempt_id 与 spec["attempt_id"],不一致返回 record_mismatch;test_foreign_attempt_is_not_assessed 断言 assesses == 0、没有归档 assessment、assessment_ref is None。✅
  • 未知 kind:只有 kind == "completed" 能进入 assess,其它值返回 outcome_rejected;test_unknown_kind_is_not_assessed 覆盖。✅
  • evidence 缺失时回路自己返回 evidence_missing / INCONCLUSIVE,不调用 assess,有对应测试。✅
  • test_missing_samples_stay_inconclusive 补了 reason 和 assessment_ref 断言;subprocess 替换改用 unittest.mock.patch;os.system 和动态 import 的边界已写在注释里。✅

当前 head 上 full-check (3.10/3.11)、installed-package (3.10/3.11)、pr-references、declarations、lint-and-smoke 全部 success;PR 描述中的 head_sha 与当前 head 一致。本次只审查测试替身回路,真实端口集成和硬件运行不在范围内。

@zengchuang-hw

Copy link
Copy Markdown
Contributor

PR review verdict — DO NOT PASS

Incremental review 45805721ca5645d41c94cf5da82ca3b3a1dba88b..c933c0ae4ed81d613bc8143d23ed304149a6e7a4. Reviewed head c933c0ae4ed81d613bc8143d23ed304149a6e7a4 and main base 5999ced639a88d359dfd7bb1798fef9548717eb5. Previous review. The original false assessment is fixed. A foreign record can still escape through the recovery and not-started paths.

本轮已解决

  • A foreign completed record and an unknown outcome kind now both stop before assessment. The new tests assert zero assessor calls and an empty assessment archive.
  • Missing evidence now returns INCONCLUSIVE without invoking the assessor. The missing-samples and subprocess tests also cover the revised behavior. Previous reviewer findings.

批准前必做(Before I can approve)

  • Dong1017, check the record's attempt binding before the recovery_required and not_started returns. I ran both with a spec bound to attempt-1 and a returned record for attempt-other. Both return their normal reason with the foreign ref in LoopResult.record_ref. A caller using that ref to read the last committed revision would follow the wrong attempt. Move the check ahead of these branches. Return an explicit mismatch without exposing the rejected ref as the current record. Add negative cases for both branches that assert no usable record or assessment ref and no assessor calls. Keep the same-attempt recovery and not-started cases green. Current branch order · Accepted handoff contract B.

非阻塞问题或建议

None this round.

I ran 16 targeted tests on this head, plus Ruff and git diff --check. All passed. The separate injected cases above reproduce the remaining binding defect. Ready CI and policy passed on this head. Please rerun both Ready full-check versions on the correction. This review covers the test-double handoff. Production ports and hardware were outside scope.

A record for another attempt is rejected before those branches and is
not returned as the current record. Assessment is still not archived.

Agent-assisted-by: Cursor Grok 4.7 (author)
Signed-off-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Dong1017 <Dong1017@users.noreply.github.com>
@cursor

cursor Bot commented Nov 10, 2026

Copy link
Copy Markdown

@zengchuang-hw @amy-why-3459 Please re-review head 27b2e388ab6a52cea8f1568c72baf0dcfb49b666. Attempt binding is now checked before recovery_required and not_started. A foreign attempt returns record_mismatch with record_ref unset, assess is not called, and no assessment is archived. test_foreign_attempt_is_rejected_for_recovery_and_not_started covers both kinds. Same-attempt recovery, not-started, the completed foreign-attempt case, and the unknown-kind case still pass. PYTHONPATH=src python3 -m unittest tests.test_architecture_closed_loop tests.test_target_module_import_gate passed, 13 + 4. Ruff passed on the touched Python files.

@amy-why-3459 amy-why-3459 left a comment

Copy link
Copy Markdown
Collaborator

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
Reviewed base 5999ced639a88d359dfd7bb1798fef9548717eb5 and head 27b2e388ab6a52cea8f1568c72baf0dcfb49b666(增量 c933c0ae4ed81d613bc8143d23ed304149a6e7a4..27b2e388ab6a52cea8f1568c72baf0dcfb49b666,1 个提交)。

  • attempt 绑定检查已移到 no_record 之后、recovery_required / not_started 之前(tests/architecture_closed_loop.py 第 134–136 行),不一致时返回 record_mismatch 且不暴露 record_ref,解决了 @zengchuang-hw 指出的外来记录从这两个分支漏出的问题。
  • test_foreign_attempt_is_rejected_for_recovery_and_not_started 用 subTest 覆盖两种 kind,断言 record_ref/assessment_ref 为空、assesses == 0、没有归档 assessment;原 completed 外来记录用例也补了 record_ref is None。同 attempt 的 recovery / not_started 用例仍然走原分支。
  • 上一轮确认过的修复(只有 completed 可进入 assess、evidence 缺失返回 INCONCLUSIVE)保持不变。

当前 head 上 full-check (3.10/3.11)、installed-package (3.10/3.11)、pr-references、declarations、lint-and-smoke 全部 success;PR 描述中的 head_sha 与当前 head 一致。本次只审查测试替身回路,真实端口集成和硬件运行不在范围内。

@Dong1017

Copy link
Copy Markdown
Collaborator Author

@zengchuang-hw dear reviewer, can you take a look 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

Incremental re-review c933c0ae4ed81d613bc8143d23ed304149a6e7a4..27b2e388ab6a52cea8f1568c72baf0dcfb49b666. Reviewed PR base 5999ced639a88d359dfd7bb1798fef9548717eb5 and head 27b2e388ab6a52cea8f1568c72baf0dcfb49b666. This resolves my previous blocking finding.

  • The attempt check now precedes recovery and not-started handling. A foreign record returns record_mismatch without exposing record_ref or assessment_ref, without calling assess, and without archiving an assessment. Same-attempt recovery and not-started retain their original reasons and record refs.
  • The completed foreign-attempt case remains rejected. Unknown, missing and empty outcome kinds do not reach assessment. Missing or malformed evidence returns INCONCLUSIVE; missing samples remain INCONCLUSIVE.
  • I reproduced both recovery/not-started leaks on the previous head, then ran the 17 closed-loop/import-gate tests and 17 additional boundary cases on the reviewed head. All passed on Python 3.12.14. Materialized review files and all 107 source Python files were checked against their Git blob SHAs.
  • Both Ready full-check jobs passed: Python 3.10 and Python 3.11, each reporting 3558 passed, 14 skipped and 286 subtests passed. CI Ruff 0.17.4 passed. These jobs tested merge ref 561a410271957b68ec859d00202b4f16f346b89d; the PR's three files and the import-gate test have identical blobs there and on the reviewed head. Installed-package checks and the required policy status also passed.

No outstanding blocking findings for this test-double slice. Full pytest and Ruff were verified through CI, not rerun locally. Production port integration, hardware runs and Issue closure remain outside this review.

Agent-assisted independent review under the zengchuang-hw account, with explicit delegation to review and approve this PR.

@Dong1017 Dong1017 added the kind:task Bounded delivery task with a delegated slice label Nov 10, 2026
@Dong1017
Dong1017 merged commit f6d2007 into main Nov 10, 2026
9 checks passed
@Dong1017
Dong1017 deleted the cursor/fake-core-loop-474c branch November 10, 2026 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:harness Harness module: profile, A/B loop, quality gates, evidence reports kind:task Bounded delivery task with a delegated slice

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants