Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions docs/development/testing-and-quality.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,35 @@ golden 来让测试通过。

### Refactor Real-Path Gate / 重构真实路径门

The PR review capability's `observable_semantics` evidence gate applies to
behavior-bearing changes, including extractions and backend migrations; it
does not infer equivalence from a refactor title. Reviewers inventory legacy
caller branches at an immutable baseline and compare the same synthetic
inputs at the exact head through the public entrypoint. Include successful
and rejected paths, full diagnostics/remediation, validation precedence,
supplied/omitted/empty/clear arguments, persisted readback, ownership and
receipts, plus replay/concurrency where relevant. A provider suite whose
implementations share the new rule is not a before/after compatibility test.

重构评审必须区分“相同判定”和“相同可观察语义”:同样拒绝但丢失对象状态或修复
提示仍是回归;新增认领前提可能阻断旧实现允许的未认领文案修正;`--note` 被 parser
接受或出现在成功响应中,也不能证明它经过 dispatch 后落盘并能独立回读。先从旧调用
方及公开契约列出合法/非法分支,再运行基线与精确 head 的对照,不从新实现生成期望。

Show regression sensitivity: the focused case must fail on the historical
defect or a deliberate dropped-field/detail or stronger-precondition mutation,
then pass on the fix. Preserve intended changes as explicit justified deltas;
do not freeze a known baseline bug or normalize away meaningful differences.
Re-review the full inventory after a correction, not only the previous finding.
Missing comparison evidence blocks an equivalence-based approval. This is a
reviewer-executed requirement projected in the packet, not a machine proof
that the comparison ran. The packet tests protect this requirement's delivery;
runtime regressions must still test actual behavior.

保留“旧缺陷/语义 mutation 失败、修复后通过”的证据;有意变更需单列理由与验证,
不能盲目追求字节一致。复审重查完整兼容清单,不沿着上一条发现自动走向批准。缺少
对照证据就不能宣称等价;packet 测试只证明要求被投影,不证明 agent 已执行或产品无缺陷。

Refactors must exercise the affected production entrypoint and real backend
before delivery. Unit tests, mocks, and in-memory conformance remain useful,
but cannot replace that proof. For changes affecting PostgreSQL authority,
Expand Down
9 changes: 7 additions & 2 deletions examples/control_plane/todo-cli-smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def main() -> int:
root = Path(tmp)
registry_path, state_file = write_fixture(root)
legacy_registry_path, _ = write_fixture(root / "legacy", register_agents=False)
claim_state_registry_path, _ = write_fixture(root / "claim-state")
claim_state_registry_path, claim_state_file = write_fixture(root / "claim-state")
original = state_file.read_text(encoding="utf-8")

blocked_todo = run_cli(
Expand All @@ -105,6 +105,7 @@ def main() -> int:
"--task-class",
"advancement_task",
)
before_rejected_claim = claim_state_file.read_text(encoding="utf-8")
blocked_claim = run_cli_error(
claim_state_registry_path,
"todo",
Expand All @@ -118,7 +119,11 @@ def main() -> int:
"--agent-id",
"codex-main-control",
)
assert "todo claim requires status=open" in blocked_claim["error"], blocked_claim
assert blocked_claim["error"] == (
f"todo claim requires status=open; todo_id '{blocked_todo['todo_id']}' "
"is status='blocked'"
), blocked_claim
assert claim_state_file.read_text(encoding="utf-8") == before_rejected_claim

bare_user_error = run_cli_error(
registry_path,
Expand Down
73 changes: 72 additions & 1 deletion loopx/capabilities/pr_review_queue/review_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def build_review_template(item: Mapping[str, Any]) -> dict[str, Any]:
_section(
"我的整体评价",
"150-300字",
"Use `code_volume`, `change_proportionality`, `default_off_isolation`, `authority_semantics`, validation results, residual risk, and exact-head freshness to state the verdict and the evidence needed for re-review.",
"Use `observable_semantics` to report baseline/head comparisons and remaining compatibility gaps; equal decision codes are insufficient. Use `code_volume`, `change_proportionality`, `default_off_isolation`, `authority_semantics`, validation results, residual risk, and exact-head freshness to state the verdict and the evidence needed for re-review.",
),
],
"review_order": _review_order(key_files),
Expand Down Expand Up @@ -179,6 +179,69 @@ def build_review_execution_contract() -> dict[str, Any]:
"automatic similarity detector or proof that a search ran."
),
},
{
"evidence_id": "observable_semantics",
"required_when": "behavior_bearing_change",
"verdict_values": [
"equivalent",
"intentional_change_validated",
"unintended_drift",
"not_yet_proven",
],
"fields": [
"baseline_revision",
"reviewed_head",
"caller_branch_inventory",
"comparison_rows",
"intentional_deltas",
"regression_sensitivity",
"unverified_dimensions",
"verdict",
],
"comparison_dimensions": [
"accepted_inputs_and_defaults",
"eligibility_and_rejection_precedence",
"full_diagnostics_and_remediation",
"argument_to_persistence_readback",
"state_receipts_and_no_effects",
"replay_and_concurrent_updates",
],
"row_fields": [
"input_and_pre_state",
"entrypoint_and_backend",
"baseline_observation",
"head_observation",
"expected_invariant_source",
"validation_evidence",
],
"rule": (
"Inventory changed and bypassed caller branches from the immutable "
"pre-change baseline, not just the new helper or PR title. Compare "
"identical synthetic inputs through the real public entrypoint and "
"affected backend at baseline and exact head. Include legal paths "
"as well as rejection paths: stricter eligibility is not inherently "
"compatible. For migrations compare before/after promotion, not "
"only multiple providers sharing the new decision. Trace supplied, "
"omitted, empty and clear argument intent through dispatch, mutation, "
"persistence and a separate readback; a success payload or parser "
"acceptance does not prove a note was saved or ownership unchanged. "
"Compare exception/exit type, full diagnostic details and remediation, "
"not just reason codes, prefixes or 'both reject'. Exercise overlapping "
"invalid conditions to check rejection precedence and assert no "
"unintended state/receipt/ownership effects. Mark non-applicable "
"dimensions with reasons. Derive expected invariants from the public "
"contract and legacy callers, never from the replacement output; "
"baseline bugs require explicitly disclosed, justified changes rather "
"than blind byte parity. Show a regression failing on the old defect "
"or a deliberate semantic mutation (dropped argument/detail, stronger "
"precondition), then passing on the fix. Normalize only documented "
"nondeterminism, never away a semantic delta. Re-review the full "
"inventory after fixes, not only the last finding. Missing baseline "
"or real-path evidence is not_yet_proven, not equivalent. This is a "
"reviewer-executed evidence gate, not automated execution or proof "
"of equivalence from CI, metadata, or a completed review template."
),
},
{
"evidence_id": "changed_line_classification",
"required_when": "always",
Expand Down Expand Up @@ -550,6 +613,7 @@ def build_review_execution_contract() -> dict[str, Any]:
"stale_head_verdict_allowed": False,
"blocking_evidence_verdicts": {
"repository_reuse": ["unjustified_duplication", "not_yet_proven"],
"observable_semantics": ["unintended_drift", "not_yet_proven"],
"change_proportionality": [
"disproportionate",
"not_yet_proven",
Expand All @@ -567,6 +631,11 @@ def build_review_execution_contract() -> dict[str, Any]:
},
"verdict_policy": {
"open_pr_blocking_finding": "REQUEST_CHANGES",
"open_pr_unresolved_semantics": (
"REQUEST_CHANGES when observable_semantics is unintended_drift or "
"not_yet_proven; equal decision codes, green suites, stricter checks "
"and resolution of the previous finding cannot establish compatibility"
),
"open_pr_unresolved_reuse": (
"REQUEST_CHANGES when repository_reuse is unjustified_duplication "
"or not_yet_proven, including missing/unverified search evidence; "
Expand Down Expand Up @@ -629,6 +698,7 @@ def build_review_plan(item: Mapping[str, Any]) -> dict[str, Any]:
required_evidence.append("typed_state_rule")
if behavior_bearing_change:
required_evidence.insert(3, "repository_reuse")
required_evidence.append("observable_semantics")
if "scope_fit" not in required_evidence:
required_evidence.append("scope_fit")
required_evidence.append("default_off_isolation")
Expand Down Expand Up @@ -659,6 +729,7 @@ def build_review_plan(item: Mapping[str, Any]) -> dict[str, Any]:
"symbol_map_required": code_change,
"scope_fit_required": behavior_bearing_change,
"repository_reuse_required": behavior_bearing_change,
"observable_semantics_required": behavior_bearing_change,
"change_proportionality_required": code_change,
"default_off_isolation_required": behavior_bearing_change,
"authority_semantics_required": code_change,
Expand Down
80 changes: 80 additions & 0 deletions tests/capabilities/test_pr_review_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def test_execution_contract_owns_deep_review_requirements() -> None:
"problem_context",
"architecture_flow",
"repository_reuse",
"observable_semantics",
"changed_line_classification",
"scope_fit",
"symbol_map",
Expand Down Expand Up @@ -121,6 +122,7 @@ def test_execution_contract_owns_deep_review_requirements() -> None:
assert contract["completion_gate"]["stale_head_verdict_allowed"] is False
assert contract["completion_gate"]["blocking_evidence_verdicts"] == {
"repository_reuse": ["unjustified_duplication", "not_yet_proven"],
"observable_semantics": ["unintended_drift", "not_yet_proven"],
"change_proportionality": ["disproportionate", "not_yet_proven"],
"default_off_isolation": ["not_isolated", "not_yet_proven"],
"authority_semantics": ["misleading", "not_yet_proven"],
Expand Down Expand Up @@ -341,3 +343,81 @@ def test_reuse_evidence_compares_semantics_beyond_the_diff() -> None:
assert "coexistence" in reuse["rule"]
assert "not an automatic similarity detector" in reuse["rule"]
assert "repository_reuse" in contract["verdict_policy"]["open_pr_unresolved_reuse"]


@pytest.mark.parametrize(
"area",
[
"product_runtime",
"app_or_ui_surface",
"ci_or_release",
"build_or_config",
"agent_instruction_surface",
"public_entry_or_policy",
],
)
def test_observable_parity_is_required_without_refactor_title_detection(
area: str,
) -> None:
item = _item(areas={area: 1})
item["title"] = "Extract shared decision helper"
item["checks"] = {"counts": {"success": 51, "failure": 0}}
plan = build_review_plan(item)
assert plan["applicability"]["observable_semantics_required"] is True
assert plan["result_template"]["evidence"]["observable_semantics"] == {
"status": "unverified"
}


@pytest.mark.parametrize("area", ["public_docs", "test_or_example"])
def test_non_behavior_changes_do_not_invent_parity_execution(area: str) -> None:
plan = build_review_plan(_item(areas={area: 1}))
assert plan["applicability"]["observable_semantics_required"] is False
assert "observable_semantics" not in plan["required_evidence_ids"]


def test_observable_semantics_covers_diagnostics_and_claim_neutral_note_paths() -> None:
contract = build_agent_response_contract()["review_execution_contract"]
parity = next(
row
for row in contract["evidence_requirements"]
if row["evidence_id"] == "observable_semantics"
)
assert parity["required_when"] == "behavior_bearing_change"
assert {
"baseline_revision",
"reviewed_head",
"caller_branch_inventory",
"comparison_rows",
"intentional_deltas",
"regression_sensitivity",
"unverified_dimensions",
"verdict",
} <= set(parity["fields"])
assert {
"accepted_inputs_and_defaults",
"eligibility_and_rejection_precedence",
"full_diagnostics_and_remediation",
"argument_to_persistence_readback",
"state_receipts_and_no_effects",
"replay_and_concurrent_updates",
} <= set(parity["comparison_dimensions"])
assert {
"input_and_pre_state",
"entrypoint_and_backend",
"baseline_observation",
"head_observation",
"expected_invariant_source",
"validation_evidence",
} <= set(parity["row_fields"])
assert parity["verdict_values"] == [
"equivalent",
"intentional_change_validated",
"unintended_drift",
"not_yet_proven",
]
assert "reviewer-executed" in parity["rule"]
assert (
"observable_semantics"
in contract["verdict_policy"]["open_pr_unresolved_semantics"]
)
18 changes: 17 additions & 1 deletion tests/control_plane/test_local_coordination_authority.py
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,22 @@ def test_real_shadow_projection_promotes_complete_complex_todo_semantics(
assert corrected_item["last_actor_agent_id"] == "agent-b"
assert not state_file.exists()

# A note-only CLI edit must survive an independent provider readback too;
# accepting the option or exercising the text branch cannot prove this.
note_command = [*correction_command[:-2], "--note", "Correction context"]
note_result = subprocess.run(
note_command, capture_output=True, text=True, check=True, timeout=30
)
assert json.loads(note_result.stdout)["ok"] is True
noted = list_goal_todos(registry_path=registry_path, goal_id="goal-a")
noted_item = next(
item for item in noted["todos"] if item["todo_id"] == "todo_claimable"
)
assert noted_item["note"] == "Correction context"
assert noted_item["text"] == "Corrected before claiming"
assert not noted_item.get("claimed_by")
assert not state_file.exists()

claim_command = [
sys.executable, "-m", "loopx.cli", "--format", "json",
"--registry", str(registry_path), "todo", "claim", "--goal-id", "goal-a",
Expand Down Expand Up @@ -606,7 +622,7 @@ def test_real_shadow_projection_promotes_complete_complex_todo_semantics(
assert created_item["text"] == "Create directly against promoted provider"
assert created_item["claimed_by"] == "agent-a"
assert after_create["authority_read"]["todo_read_model"]["todo_count"] == 4
assert claimed_item["note"] == claimable["note"]
assert claimed_item["note"] == "Correction context"

# Real CLI, no Markdown file: provider data feeds an in-memory editor and
# only requested fields return through TS CAS. Complex sibling fields do
Expand Down