Repository navigation
test: add a fake prepare-to-assess handoff loop - #377
Conversation
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>
|
@zengchuang-hw @amy-why-3459 Please review head |
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Reviewed base 5999ced and head 4580572 (full diff, first review).
Verified:
tests/architecture_closed_loop.pyis the test-local handoff named indocs/module-layout.md. Its imports are__future__,hashlib,json, anddataclasses. The diff leaves Knowledge, Harness, Benchmark, and Bundle revision untouched, and the note keeps #371 as the Apps proposal.bind_executionreturns None when the re-read prepared bytes do not hash toPreparedRef.digest, and whenattempt_idis missing or empty, sorun_loopreturnsbind_rejectedbeforeharness.execute(test_bind_rejects_a_digest_mismatch_without_writing_records).- Prepared-archive failure returns
archive_prepared_failedwithexecutes == 0.no_recordand an unparsable record returnno_recordwith no assessment ref.not_startedandrecovery_requiredreturn beforebenchmark.assess;recovery_requiredleavescancelledfalse. - Assessment-archive failure keeps
record_refand the in-memory verdict and leavesassessment_refunset. The injected fake maps missing samples toINCONCLUSIVE. 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 frominferrsi-json-v1.test_coordinator_does_not_import_production_coresrejects AST imports whose module name starts withinferrsi.
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.
|
PR review verdict — DO NOT PASS Reviewed 批准前必做(Before I can approve)
非阻塞问题或建议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 |
|
本次 review 范围:完整 diff(新 PR),base 阻塞项
非阻塞建议
修好后请在新 head 上重跑两版 |
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>
|
@zengchuang-hw @amy-why-3459 Please re-review head |
amy-why-3459
left a comment
There was a problem hiding this comment.
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 一致。本次只审查测试替身回路,真实端口集成和硬件运行不在范围内。
|
PR review verdict — DO NOT PASS Incremental review 本轮已解决
批准前必做(Before I can approve)
非阻塞问题或建议None this round. I ran 16 targeted tests on this head, plus Ruff and |
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>
|
@zengchuang-hw @amy-why-3459 Please re-review head |
amy-why-3459
left a comment
There was a problem hiding this comment.
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 一致。本次只审查测试替身回路,真实端口集成和硬件运行不在范围内。
|
@zengchuang-hw dear reviewer, can you take a look when you have a chance? |
zengchuang-hw
left a comment
There was a problem hiding this comment.
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_mismatchwithout exposingrecord_reforassessment_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 remainINCONCLUSIVE. - 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-checkjobs 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 ref561a410271957b68ec859d00202b4f16f346b89d; 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.
Governing Issue and delivery slice
Refs #372
Refs #362
5999ced639a88d359dfd7bb1798fef9548717eb5. A clean scan is not module isolation.tests/architecture_closed_loop.pyis called bytests/test_architecture_closed_loop.pywith 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 kindcompletedfor the bound attempt may be assessed.not_startedand 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 staysINCONCLUSIVE. Missing evidence returnsINCONCLUSIVEwithout calling assess.knowledge/,benchmark/, orapps/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.Changed artifacts and evidence
tests/architecture_closed_loop.pyPYTHONPATH=src python3 -m unittest tests.test_architecture_closed_loop tests.test_target_module_import_gate: 13 + 4 passedrecord_mismatchwithrecord_refunset, before recovery, not-started, and assesstest_foreign_attempt_is_not_assessedandtest_foreign_attempt_is_rejected_for_recovery_and_not_started. Same-attempt recovery and not-started still passcompletedis assessedoutcome_rejectedbefore assesstest_unknown_kind_is_not_assesseduses kindfinishedwith a parseable recordimportlib.import_modulegit diff --checkcleanChecks and dependencies
tests/so it does not take that path.Remaining work and Issue closure
search_knowledge, archive ownership versus RunStore, and the Apps directory name.Handoff (only when an actual handoff or decision changed)
27b2e388ab6a52cea8f1568c72baf0dcfb49b666.