diff --git a/AGENTS.md b/AGENTS.md index 07f47f5473..ed561b844f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -245,6 +245,11 @@ gates. Apply the whole-viewport attention review in `docs/development/design.md` ("Earn The User's Attention") before implementation and in PR visual evidence. +For production-bound interactive UI changes, carry one ordinary user task through +`docs/development/frontend-delivery.md`: compare the shortest authorized journey +before coding, validate the packaged interaction and owning state, and distinguish +proposed changes from installed behavior. For documentation and prototypes, name +the simulated boundary without claiming packaged or installed readback. Before changing or reproducing any LoopX UI, read and follow the repository-root `docs/development/design.md`. This includes websites, dashboards, desktop applications, diff --git a/docs/development/design.md b/docs/development/design.md index c2c25bc5b4..fe77acb5b7 100644 --- a/docs/development/design.md +++ b/docs/development/design.md @@ -260,6 +260,12 @@ bounded horizontal scrolling. ## UI Reproduction Workflow +For feature development, carry one user task through the +[task-first delivery workflow](frontend-delivery.md#task-first-delivery--从用户任务开始). +A visually consistent composition must also eliminate unnecessary re-entry and +confirmation, identify the affected target, and show truthful state after action. + + For UI implementation, migration, or reproduction: 1. Read this file before editing. diff --git a/docs/development/frontend-delivery.md b/docs/development/frontend-delivery.md index 7b33679139..9bb6158a5d 100644 --- a/docs/development/frontend-delivery.md +++ b/docs/development/frontend-delivery.md @@ -6,6 +6,82 @@ validation only; resolving a source merge followed by a build produces one coherent HTML/JavaScript/CSS set. Do not concatenate competing HTML entrypoints or asset-retention manifests when resolving an older PR. +## Task-first delivery / 从用户任务开始 + +Use the existing task/PR delivery brief, not a second checklist or approval form. +Production-bound interactions require packaged task and state readback. For a +prototype or documentation example, compare the proposed journey but label its +simulated boundary instead of claiming that the interaction is installed. +Before implementation, name one ordinary request, its current failure, the +observable result, affected entrypoints and existing state/effect owner. Derive +acceptance from that request, a demonstrated failure and the nearest accepted +product contract, before designing components or copying the current click +sequence into tests. Reuse a relevant, accepted +[Golden Query](../product/use-cases/steward/golden-queries.md) when one covers the +task; proposed or unrelated queries can inform exploration but are not a +mandatory acceptance source. If none applies, the task, failure and owning +contract still define acceptance. Inspect current main and related work first. +A passing implementation-shaped test is not an independent experience oracle. + +Carry the same task through these steps: + +1. **Compare the journey.** Write the current and proposed user steps in the + existing before/after evidence. Count required navigation, information the + user must repeat, questions, confirmations and recovery effort. Explain the + information, authority or unavoidable prerequisite supplied by each required + step; remove a step that supplies none. This is a comparison, not a universal + click limit. Keep necessary scoped consent and explicit effect confirmation; + make optional refinement optional. A complete conversational draft should not + require a second form just to repeat the same facts. +2. **Choose the owner and composition.** Reuse the existing typed state and + action owner and inspect companion App/CLI/messaging paths. Follow + [design.md](design.md#earn-the-users-attention) for the whole viewport, not + individual cards. Navigation follows user purpose; clearly show the target + and source of a setting. Do not expose storage scopes as competing pages or + list controls that cannot apply to the selected target. +3. **Implement and challenge.** Use the smallest meaningful regression derived + from the accepted outcome. Select realistic populated data and one negative + or recovery path likely to falsify the claim: multiple owners, out-of-order + timestamps, stale/disconnected state, duplicate click, cancellation or reload. + Select by the touched risk, not all cases for every patch. Reuse shared TS + projections; fix the owning selector rather than sorting an already-sorted + mock or adding a frontend-only source of truth. +4. **Walk the actual product.** Start at the affected user entry, use the built + packaged surface, inspect the rendered viewport, perform the task and read + back the result. Validate affected narrow/keyboard/language behavior. Pair + browser fixtures with the real owning backend when they replace the rule + being claimed. Use disposable synthetic state; do not alter active Goals. + Record source/build identity and fixture boundaries. Source preview, packaged + browser, installed App and real model each prove different things. +5. **Review the task before the diff narrative.** The review capability's + `problem_context.outcome_impact.user_experience` references the same journey + and validation, then asks what remains wrong even if every test passes. + Reused evidence needs its revision and unchanged assumptions. A material + unsupported claim remains unproven. A necessary confirmation can be accepted + with its authority basis; green CI cannot justify redundant confirmation. +6. **Deliver and read back.** Report the observable improvement, evidence limits + and whether code is proposed, merged, packaged or installed. After an + authorized upgrade, reopen the exact installed surface and check the same + task; a pushed PR does not repair an already-running App. Use existing + install/release owners and retain the previous working version for rollback. + +Public PR evidence should fit in the existing summary/validation: one short +before/after journey, a screenshot or clip for the affected visual composition, +and the state readback plus tested revision and limits. Keep private messages, +local paths and raw run logs out. The existing public first-screen preview gate +still applies; do not add user approvals for routine implementation decisions. + +**Evaluation cadence:** ordinary work runs offline regressions and affected +browser/backend checks. Paid model interpretation and review-decision probes +belong to release-candidate qualification, with failures/skips retained. A +scripted browser passing cannot certify model intent recognition; an unrun live +probe is not a pass. See [release readiness](../product/release-readiness.md). + +中文要点:先定义用户想完成的事,再比较最短合法路径;复用已有授权与状态归属, +减少重填、重复确认和页面跳转。以真实构建页面、代表性数据和状态回读验收,区分 +模拟边界、打包验证和安装生效。评审先挑战交互成本与证据,再解释代码;平时不跑 +付费模型评测,不用测试数量或 PR 合并代替用户任务完成。 + ## Source development and updates Prepare Python with `uv sync --extra test`, then run from the repository root: diff --git a/examples/fixtures/pr-review-history/README.md b/examples/fixtures/pr-review-history/README.md index 793a2ebec7..f75dda90c6 100644 --- a/examples/fixtures/pr-review-history/README.md +++ b/examples/fixtures/pr-review-history/README.md @@ -54,11 +54,17 @@ Run from the checkout root: ```sh uv run --extra test python -m pytest tests/capabilities/test_pr_review_body.py -q -# Requires the normal process-only provider credentials; no tools or writes. +# Release qualification only; normal process-only credentials, no tools or writes. LOOPX_REVIEW_LIVE_TEST=1 uv run --extra test python -m pytest \ tests/capabilities/test_pr_review_behavior.py -k historical -q ``` +The same release-only runner also contains paired synthetic frontend cases for +redundant confirmation, recent-completion ordering and settings target clarity. +Run the full `test_pr_review_behavior.py` suite without `-k historical` to include +them at release qualification. Routine PRs run offline checks with live calls +disabled. Positive twins prevent a blanket rejection of forms or confirmation. + Live results qualify reasoning over supplied evidence only. They do not demonstrate autonomous repository investigation, rerun the old provider concurrency tests, or establish an improvement over a baseline model. No diff --git a/loopx/capabilities/pr_review_queue/README.md b/loopx/capabilities/pr_review_queue/README.md index 8116387efc..0b89d59ebd 100644 --- a/loopx/capabilities/pr_review_queue/README.md +++ b/loopx/capabilities/pr_review_queue/README.md @@ -63,6 +63,19 @@ continuation, retry/restart, competing work, truthful status, intervention cost and correction/recovery. Reuse walkthroughs and validation references rather than producing a second report or requiring a soak for every change. +Policy revision 12 makes that journey comparison concrete within the existing +fields: compare required steps with the shortest valid authorized path, justify +each mandatory question/confirmation, inspect populated whole-view composition, +and distinguish fixture, packaged and installed evidence. No new result field, +UI-only capability or second review authority is added. The checker still +validates declarations rather than usability; the reviewer must execute and +inspect the evidence. The paired frontend cases in `test_pr_review_behavior.py` +challenge unnecessary forms, misleading recency and scope-confused settings, +with positive controls preserving legitimate consent. These reasoning probes +run only for release qualification; offline checks do not certify model quality. +LoopX authors use the [frontend delivery workflow](../../../docs/development/frontend-delivery.md); +reviews of other repositories follow their own product contracts. + Legitimate safety, budget and dependency waits can be an `accepted_tradeoff` with an independent acceptance basis and bounded cost/recovery. Do not weaken authority to manufacture progress. A scoped `not_applicable` still names the diff --git a/loopx/capabilities/pr_review_queue/review_contract.py b/loopx/capabilities/pr_review_queue/review_contract.py index f9d9bd4856..f42414e7c3 100644 --- a/loopx/capabilities/pr_review_queue/review_contract.py +++ b/loopx/capabilities/pr_review_queue/review_contract.py @@ -7,7 +7,7 @@ from .review_body import REQUIRED_FINAL_SECTIONS, review_body_requirements # Increment when review requirements change without changing the packet shape. -REVIEW_POLICY_REVISION = 11 +REVIEW_POLICY_REVISION = 12 # A red check is an observation, not evidence that the reviewed PR caused it. # This contract belongs to review judgment; merge readiness still owns whether @@ -54,6 +54,21 @@ "For user_experience, compare the real affected CLI, UI or messaging journey: setup and " "repeated intervention, truthful state/readback, actionable failure, correction/cancel and " "recovery. Inspect existing companion surfaces; a backend success is not a usable journey. " + "For an affected user journey, establish the ordinary task and expected result before " + "reading the implementation path. In existing before_after and walkthrough evidence, " + "compare required navigation, re-entry of known information, clarification, confirmation " + "and recovery steps with the shortest valid path through the existing authority boundary. " + "Explain what new information or authority each mandatory step supplies; a form that " + "works can still be redundant. Optional editing is not a mandatory decision. Do not " + "remove necessary scoped consent or correctness checks to minimize clicks. " + "For visible changes inspect the rendered whole viewport with representative populated " + "data, not only component presence: target/owner, hierarchy, truthful freshness/order " + "and the next action must be understandable together. Follow one risk-relevant " + "correction, cancel, reload or unavailable path through state readback. Identify source " + "revision, built/installed surface and any mocked boundary in existing evidence refs. " + "A fixture that supplies fresh sorted data cannot prove production freshness or sorting; " + "a screenshot cannot prove persistence or adoption. Reuse unchanged evidence with an " + "invalidation check; do not demand every viewport/state or paid model calls per PR. " "Reuse concrete walkthrough and validation references; select bounded cases by changed " "risk rather than requiring a long soak or every surface for every PR. Derive expected " "outcomes from the accepted product contract, not from the patch. A deliberate safety, " diff --git a/skills/loopx-self-repair/references/repair-patterns.md b/skills/loopx-self-repair/references/repair-patterns.md index 4252df3cea..acb8aa2fc7 100644 --- a/skills/loopx-self-repair/references/repair-patterns.md +++ b/skills/loopx-self-repair/references/repair-patterns.md @@ -13,7 +13,7 @@ teaches a reusable control-plane lesson. | `acceptance_hold_recovery_selection_split` | Newly created advancement work is acceptance-unbound, repeated vision replans never expose its hold, or a replan packet also selects an unrelated due monitor. | Canonical acceptance tasks, scoped source Todos, bounded trigger checkpoints, effective action and original Turn receipt. | Recovery covered stale associations only; generic vision gaps displaced hold identities; candidate inventory leaked into the selected execution target. | Route missing and stale associations through the existing bounded replan lane, retain exact hold checkpoints before generic gaps, and separate replan from candidate selection. Keep owner association and completion validation enforced. A new unbound repair Todo is not runnable recovery; only a qualified successor or concrete blocker settles the exact hold. Validate real File/SQLite CLI paths and receipt reentry without mutating an active Goal. | | `acceptance_validation_failure_flattened` | A bound Todo is `ready` and its ordinary validator passes, yet completion reports only `goal_acceptance_validation_rejected`; the agent searches contract bindings before discovering a workspace or runner failure. | Exact Todo acceptance state, completion's typed criterion failure, recorded delivery workspace, current worktree cleanliness, and the privacy-safe runner receipt. | The completion boundary collapsed a failed criterion receipt into a generic contract error, hiding the workspace or command status. | Project the failed criterion ID, allowlisted validation status, safe exit code and bounded next action without command output or local paths. Repair the execution context and retry under the same Turn/lease; do not rebind owner criteria or infer a Goal-wide hold. | | `review_compatibility_assumption_gap` | A correct fix retains parallel protocol paths, and the review treats historical receipt recovery as proof that every old request decoder is needed. | Published review, structured compatibility rationale, real caller/deployment inventory, stored request versus receipt shape, and smaller-design readback. | Re-review verifies the last bug but does not separate compatibility obligations or test consolidation; free-text claims pass as evidence. | Replace the capability's existing compatibility rationale with a bounded structured assessment. Distinguish transient requests, independent client rollout and persisted replay formats; compare one typed current contract; preserve real legacy consumers. Cover needless retention and unsafe removal with positive twins. Keep optional simplifications advisory and do not make field completeness certify evidence truth. | -| `review_outcome_continuity_gap` | Local feature checks pass while later work starves, recovery only records a blocker, or ordinary users face new repeated intervention. | Accepted product outcome, published review, real later invocations, affected user surfaces, durable progress and authorized recovery. | Review proved one operation or prevention of bypass without checking sustained progress and the complete user journey. | Make long-horizon continuity and user experience explicit judgments in the existing problem context. Reuse bounded real walkthroughs; apply scope counterexamples when gates are involved. Accept deliberate waits only with an independent basis and recovery or safe terminal route. Qualify with exact-source historical counterexamples and repaired positive controls; keep old verdicts out of model inputs and distinguish reported historical checks from newly executed tests. Section length and declared evidence cannot certify truth. | +| `review_outcome_continuity_gap` | Local feature checks pass while later work starves, recovery only records a blocker, or ordinary users face new repeated intervention. | Accepted product outcome, published review, real later invocations, affected user surfaces, durable progress and authorized recovery. | Review proved one operation or prevention of bypass without checking sustained progress and the complete user journey. | Make long-horizon continuity and user experience explicit judgments in the existing problem context. Reuse bounded real walkthroughs; apply scope counterexamples when gates are involved. Accept deliberate waits only with an independent basis and recovery or safe terminal route. Qualify with exact-source historical counterexamples and repaired positive controls; keep old verdicts out of model inputs and distinguish reported historical checks from newly executed tests. Section length and declared evidence cannot certify truth. For frontend friction, compare the shortest authorized task path before encoding the UI sequence; inspect populated composition and real state readback. Record fixture/build/install boundaries and repair the existing delivery workflow, not a second checklist. | | `source_context_legacy_downgrade` | An unstamped record mutates state through a legacy path after the authoritative source disappears, although another record in the same owner context preserves an exact source identity. | Selected record identity, all same-context durable candidates, source path availability, operation entrypoint, and business-file bytes before and after refusal. | The adapter treated one selected record's missing identity as proof that the whole context was legacy and ignored exact historical evidence when the current authority could not be read. | Resolve source-context availability once for selection and mutation. Exact history may prove that legacy fallback is unsafe, but it must never supply or guess the current identity. Fail closed before writes when authority is unavailable; preserve contexts with no exact history through opposite legacy controls across every mutation entrypoint. | | `archive_capture_classification_gap` | Whole-Goal capture rejects a reachable archived Agent record although its active read was valid. | Recorded role/class, existing legacy read classification, transitive dependency closure, bootstrap and writer-outbox readback. | Archive storage preserved the role but omitted the resolved class; capture treated missing class as missing authority. | Keep recorded identity separate from compatibility classification. Only a recorded Agent role can adopt the existing read class; use it consistently for closure and materialization. Preserve the class on new archive moves, keep user authority fail-closed, and validate full source capture in disposable real providers without changing the active Goal. | | `shadow_proof_transport_amplification` | A large Goal cannot capture its first mutation or finish drain although the same tiny Goal succeeds; RPC rejects an oversized response. | Same source population, base/head serialized response sizes, actual sequence/drain callers, qualified lineage and cursor readback. | A consumer needing progress or partition markers received the full head and every historical projection across the language boundary. | Keep full history verification in the typed owner and return a purpose-specific compact proof. Preserve receipts, sequence and lineage checks; do not raise transport limits, truncate source records or weaken qualification to make the test pass. Cover the old oversized response and real CLI capture, drain and reviewed cutover on a disposable snapshot. | diff --git a/tests/capabilities/test_pr_review_behavior.py b/tests/capabilities/test_pr_review_behavior.py index 60c7f22865..a21d5b454e 100644 --- a/tests/capabilities/test_pr_review_behavior.py +++ b/tests/capabilities/test_pr_review_behavior.py @@ -1,7 +1,7 @@ """Small no-tools decision probes; oracles are not exposed to the model. These test review reasoning on supplied evidence, not repository investigation. -Live execution is opt-in and uses the existing bounded provider transport. +Live execution is release-only opt-in and uses the existing bounded provider transport. """ from __future__ import annotations @@ -267,6 +267,70 @@ ]) +# Product acceptance supplies the oracle; passing the author's click script does not. +# Synthetic paired cases, not claims that historical UI/backend tests ran here. +FRONTEND_CASES = [ + ( + {"request": "Review conversational goal preparation in the existing App.", + "problem": "A complete draft should reach one scoped creation preview and explicit apply; optional editing must remain available.", + "proposal": "Every complete draft opens a mandatory form, then another confirmation, then the existing preview and apply. All fields already exist in the draft.", + "evidence": "The packaged click script follows that sequence and passes; screenshots are readable. Neither extra step supplies new facts, missing permissions or consent. No test compares the accepted direct-preview journey. Backend validation and duplicate-submit tests pass."}, + "REQUEST_CHANGES", "architecture", + ), + ( + {"request": "Review conversational goal preparation in the existing App.", + "problem": "A complete draft should reach one scoped creation preview and explicit apply; optional editing must remain available.", + "proposal": "Complete drafts open the existing typed preview directly; edit is optional and incomplete drafts ask for the missing consequential input. Apply remains explicit.", + "evidence": "Packaged task walkthrough and real action readback show one preview/apply, no effect before apply, safe cancel, changed-input revalidation and reload without duplicate creation. Source identity is recorded. Tests cover complete and incomplete inputs rather than enforcing one fixed click script; all other evidence is verified."}, + "APPROVE", "none", + ), + ( + {"request": "Review a recent-completions panel.", + "problem": "Users should see the most recently completed items across projects, ordered by completion time.", + "proposal": "Concatenate project arrays and take the first three items. Show the result as recent completions.", + "evidence": "The browser fixture supplies project arrays already ordered with newest completions first; screenshot and click tests pass. With project A completed on March 1 before project B completed on March 3, the actual selector still shows A first. Refresh time is displayed as the completion time. The interface renders clearly but no real selector ordering/readback comparison passes."}, + "REQUEST_CHANGES", "architecture", + ), + ( + {"request": "Review a recent-completions panel.", + "problem": "Users should see the most recently completed items across projects, ordered by completion time.", + "proposal": "The existing shared projection orders known completion times across projects, uses a stable tie break and labels unknown dates without inventing freshness. UI renders that projection.", + "evidence": "Disposable real backend records intentionally invert insertion, project and completion order; projection and packaged page agree before and after reload. Equal timestamps and unknown times preserve explicit semantics. Populated/narrow views show readable titles and dates. No frontend-only ordering owner is added and other required checks pass."}, + "APPROVE", "none", + ), + ( + {"request": "Review a settings catalog exposing machine and per-project options.", + "problem": "The accepted settings scope contract requires users to know which project is being changed and see only options applicable to that target. No steward Golden Query covers this task.", + "proposal": "Offer two similarly named capability tabs; both list all options. Selecting a machine-only option in a project shows a read-only warning. Preserve the previous preview when switching project.", + "evidence": "Components and screenshots pass on an empty single-project fixture. The populated two-project walkthrough shows the old target's preview after switching, and users must open each option to learn its scope. Backend rejects wrong-target writes, but the UI does not identify the actionable target before confirmation."}, + "REQUEST_CHANGES", "architecture", + ), + ( + {"request": "Review a settings catalog exposing machine and per-project options.", + "problem": "The accepted settings scope contract requires users to know which project is being changed and see only options applicable to that target. No steward Golden Query covers this task.", + "proposal": "Reuse one catalog with an explicit target, visible effective source and purpose-based navigation. Filter by the existing scope contract; target changes clear draft/preview and fetch the selected state.", + "evidence": "The packaged two-project walkthrough verifies visible target, keyboard selection, narrow layout and an unavailable target. Real revision-checked preview/apply/readback affects only the selected target. Cancel and reload preserve saved settings, not an abandoned draft. Legitimate effect confirmation remains; other evidence is verified."}, + "APPROVE", "none", + ), +] +CASES.extend(FRONTEND_CASES) + + +def test_non_steward_settings_acceptance_does_not_require_a_golden_query(): + guide = (Path(__file__).parents[2] / "docs/development/frontend-delivery.md").read_text() + task_first = " ".join(guide.split("## Task-first delivery", 1)[1] + .split("## Source development", 1)[0].split()) + assert "nearest accepted product contract" in task_first + assert "when one covers the task" in task_first + assert "not a mandatory acceptance source" in task_first + settings = [(scenario, verdict) for scenario, verdict, _ in FRONTEND_CASES + if scenario["request"] == "Review a settings catalog exposing machine and per-project options."] + assert [verdict for _, verdict in settings] == ["REQUEST_CHANGES", "APPROVE"] + assert all("No steward Golden Query covers this task" in scenario["problem"] + and "accepted settings scope contract" in scenario["problem"] + for scenario, _ in settings) + + def test_decision_procedure_is_in_the_real_packet_before_prose(): response = build_agent_response_contract() assert response["review_execution_contract"]["decision_procedure"]["order"] == [ diff --git a/tests/capabilities/test_pr_review_result_check.py b/tests/capabilities/test_pr_review_result_check.py index bbf89e800c..92960fd2cf 100644 --- a/tests/capabilities/test_pr_review_result_check.py +++ b/tests/capabilities/test_pr_review_result_check.py @@ -220,8 +220,9 @@ def test_local_goal_achievement_cannot_hide_material_outcome_impact(dimension, d assert check_review_result(packet, result)["ok"] -def test_legitimate_wait_needs_acceptance_basis_and_bounded_recovery(): - packet, result, impact = _outcome_review("long_horizon") +@pytest.mark.parametrize("dimension", ["long_horizon", "user_experience"]) +def test_legitimate_wait_needs_acceptance_basis_and_bounded_recovery(dimension): + packet, result, impact = _outcome_review(dimension) impact["decision"] = "accepted_tradeoff" assert not check_review_result(packet, result)["ok"] impact.update(