Skip to content

docs(pr138): retrofit acceptance-pack evidence for evidence-first review gate - #143

Closed
mindfn wants to merge 6 commits into
mainfrom
codex/pr138-evidence-pack-retrofit
Closed

docs(pr138): retrofit acceptance-pack evidence for evidence-first review gate#143
mindfn wants to merge 6 commits into
mainfrom
codex/pr138-evidence-pack-retrofit

Conversation

@mindfn

@mindfn mindfn commented Mar 2, 2026

Copy link
Copy Markdown
Collaborator

Context

PR #138 was prepared under the previous evidence format. This PR provides the incremental retrofit so #138 can be compared against the new Evidence-first review requirements.

Scope

  • Documentation-only retrofit on:
    • docs/features/agentscope-d7-plan-state-tools.md
  • No Python/runtime code changes.

Retrofit items added

  • ### Contract Delta (schema / error_code / retry)
  • ### Golden Cases
  • ### Regression Summary (runner commands + pass/fail/skip)
  • ### Observability and Failure Localization (start/tool_call/end/fail + locator fields)
  • ### Structured Review Report
  • Intent/Implementation PR markers in review links

Validation notes

Why this PR

This keeps the comparison clean:

  1. feat(plan_v2): implement D7 plan state machine and revise/finish tools #138 original (old evidence style)
  2. docs(pr138): retrofit acceptance-pack evidence for evidence-first review gate #143 incremental evidence retrofit (new-rule style content)

mindfn added 5 commits March 2, 2026 14:28
Deliver AgentScope D7 slice by introducing explicit plan/step lifecycle state management in plan_v2, adding missing plan revision/finish tools, and syncing docs/OpenSpec evidence under documentation-first workflow.

Key changes:
- add plan_v2 lifecycle state model (todo/in_progress/done/abandoned) with transition guards in types.py
- add compatibility synchronization between step statuses and completed_step_ids
- add ReviseCurrentPlanTool and FinishPlanTool with deterministic validation and terminal-state rules
- update CreatePlanTool/ValidatePlanTool/SubAgentTool to drive state transitions and keep critical_block aligned
- update planner tool registration, plan_v2 exports, and plan prompt guidance for revise/finish workflow
- add D7 regression test matrix in tests/unit/test_plan_v2_tools.py
- add docs-first artifacts: plan_v2 module design doc, D7 feature evidence doc, OpenSpec proposal/design/specs/tasks, and TODO claim/evidence sync

Verification:
- /Users/lang/workspace/github/Deterministic-Agent-Runtime-Engine/.venv/bin/pytest -q tests/unit/test_plan_v2_tools.py
- /Users/lang/workspace/github/Deterministic-Agent-Runtime-Engine/.venv/bin/pytest -q tests/unit/test_plan_v2_tools.py tests/unit/test_react_agent_gateway_injection.py tests/unit/test_dare_agent_step_driven_mode.py
- /Users/lang/workspace/github/Deterministic-Agent-Runtime-Engine/.venv/bin/pytest -q
- openspec validate --changes "agentscope-d7-plan-state-tools"
- openspec instructions apply --change "agentscope-d7-plan-state-tools" --json
- ./scripts/ci/check_governance_evidence_truth.sh

Rationale:
This closes Gap-P1/P2/P3 in the AgentScope migration backlog while preserving runtime compatibility. Explicit lifecycle states and finish/revise tools remove ambiguous progress tracking and make critical-block guidance auditable and deterministic.
Update D7 feature and claim ledgers after PR creation so governance records match current execution state.

Changes:\n- mark D7 feature doc as in_review and add PR/review links\n- update D7 claim notes in overall/domain TODO ledgers from pending PR to PR #138 in review\n- keep claim scope and owner mapping unchanged
sync_completed_step_ids previously accessed step.status and step.step_id directly, which can raise AttributeError when PlannerState is restored with legacy Step-like objects missing status.\n\nThis change introduces Step-like access helpers with dict/object fallback, then rebuilds completed_step_ids using normalized state and safe step_id extraction. Invalid/blank step ids are ignored instead of crashing the sync path.\n\nA regression test covers mixed modern + legacy + dict-backed steps to ensure sync remains stable and only includes valid terminal step ids.
The design-doc-drift CI gate enforces that every docs/design/modules/*/README.md includes the section header:\n"## 能力状态(landed / partial / planned)".\n\nThe plan_v2 module README lacked this section, causing ci-gate/design-doc-drift to fail on PR #138.\n\nThis commit adds the required section and documents landed/partial/planned status for plan_v2 so governance checks can validate module status consistency.
A new Codex review item on PR #138 identified that transition_step read step.status directly, which can raise AttributeError for restored/pre-D7 step objects lacking a status attribute.\n\nThis change aligns transition_step with the existing compatibility read path by using _step_state() for current-state evaluation, broadens get_step() lookup to use _step_id() fallback, and writes the normalized status via dict/object-compatible assignment.\n\nA regression test now verifies that transitioning a legacy step without status succeeds and persists the new lifecycle state.
@mindfn
mindfn force-pushed the codex/pr138-evidence-pack-retrofit branch from 7053e68 to c0733f7 Compare March 2, 2026 07:40
@mindfn
mindfn changed the base branch from codex/agentscope-d7-plan-state-tools to codex/evidence-first-gates-v2 March 2, 2026 07:40
@mindfn mindfn changed the title docs(pr138): retrofit acceptance-pack evidence for evidence-first review gate stack(pr138): rebase D7 branch on #142 gate and retrofit acceptance-pack evidence Mar 2, 2026
@mindfn

mindfn commented Mar 2, 2026

Copy link
Copy Markdown
Collaborator Author

Rebase/update complete.

Validation on current head (c0733f7) after rebasing onto codex/evidence-first-gates-v2:

./scripts/ci/check_governance_evidence_truth.sh

Output summary:

  • checking docs/features/agentscope-d7-plan-state-tools.md
  • checking docs/features/enhance-doc-governance-traceability.md
  • skip non-governed docs/features/p0-default-eventlog.md (draft)
  • skip non-governed docs/features/p0-step-driven-execution.md (done)
  • passed

So this branch is now validated against #142 gate rules, not the old gate behavior.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0733f794c

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +165 to +166
normalized_next = _normalize_state(next_state, default="todo")
if not is_valid_state_transition(current, normalized_next):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject invalid lifecycle states instead of coercing to todo

transition_plan and transition_step normalize next_state with _normalize_state(..., default="todo"), so an invalid value can be silently accepted as a no-op when the current state is todo (for example, transition_step("s1", "don") becomes todo -> todo and succeeds). This violates the new lifecycle contract that illegal transitions must be rejected deterministically and makes caller/state-machine bugs hard to detect.

Useful? React with 👍 / 👎.

Upgrade PR #138 feature evidence to the new Evidence-first acceptance-pack contract without changing runtime implementation.

Key changes:

- Add Contract Delta section (schema/error_code/retry semantics).

- Add Golden Cases section with concrete updated files.

- Add Regression Summary with runner commands and pass/fail/skip totals.

- Add Observability and Failure Localization section with start/tool_call/end/fail markers and locator fields.

- Add Structured Review Report covering module boundaries, new state, concurrency/timeout/retry, side effects, and residual risk.

- Add Intent PR and Implementation PR markers in review links to satisfy merge-gate link contract.

Rationale: make #138 reviewable under the new gate model and provide side-by-side comparison with old evidence style.
@mindfn
mindfn force-pushed the codex/pr138-evidence-pack-retrofit branch from c0733f7 to 7a20662 Compare March 2, 2026 08:42
@mindfn mindfn changed the title stack(pr138): rebase D7 branch on #142 gate and retrofit acceptance-pack evidence docs(pr138): retrofit acceptance-pack evidence for evidence-first review gate Mar 2, 2026
@mindfn
mindfn changed the base branch from codex/evidence-first-gates-v2 to codex/agentscope-d7-plan-state-tools March 2, 2026 08:43
@mindfn

mindfn commented Mar 2, 2026

Copy link
Copy Markdown
Collaborator Author

Correction applied:

Reason: stacked-on-#142 view pulled in D7 code commits into PR diff, which breaks the intended pure retrofit comparison.

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/blob/7a2066285c6acb0c71b32c725050ed92d9337b9a/dare_framework/plan_v2/tools.py#L345
P1 Badge Handle legacy dict steps when revising plan state

ReviseCurrentPlanTool.execute assumes every existing step has attribute access (step.step_id), but PlannerState now explicitly supports legacy Step-like objects/dicts elsewhere. If a persisted plan contains dict steps (e.g., {"step_id": ..., "status": ...}), calling revise_current_plan raises AttributeError instead of returning a deterministic tool error, which breaks revision for migrated sessions.


https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/blob/7a2066285c6acb0c71b32c725050ed92d9337b9a/dare_framework/plan_v2/tools.py#L442
P1 Badge Avoid crashing finish_plan on dict-based legacy steps

FinishPlanTool.execute builds pending via step.step_id after _pending_steps, so plans containing legacy dict steps crash with AttributeError during finish_plan instead of returning a ToolResult. This makes terminal closure (done/abandoned) impossible for migrated plan states and violates the deterministic error-handling behavior expected from plan tools.

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@mindfn

mindfn commented Mar 2, 2026

Copy link
Copy Markdown
Collaborator Author

补充:按“138代码 + 142规则 + 143文档补齐”的集成诉求,我新开了 stacked 集成 PR:#146

这样可以同时保留:

  1. docs(pr138): retrofit acceptance-pack evidence for evidence-first review gate #143 纯文档增量对比视角
  2. stack(pr138): integrate D7 implementation on top of evidence-first gate #146 新规则下的完整集成视角

@mindfn
mindfn force-pushed the codex/agentscope-d7-plan-state-tools branch 3 times, most recently from 1b34033 to 7eb8cdb Compare March 3, 2026 06:16
Base automatically changed from codex/agentscope-d7-plan-state-tools to main March 3, 2026 11:05
@mindfn mindfn closed this Mar 3, 2026
mindfn added a commit that referenced this pull request Mar 4, 2026
Update D7 lifecycle status across project/agentscope boards and feature aggregation evidence to reflect merged delivery.\n\nChanges:\n- Mark CLM-20260302-D7 and CLM-20260302-AG3 as done with merged PR evidence (#138).\n- Align D7 priority table/stage annotations from active closeout to completed baseline.\n- Promote feature doc docs/features/agentscope-d7-plan-state-tools.md to status=done and add review/merge links (#138/#143/#146).\n- Activate next fix claim CLM-20260304-AG10 for T0-6 (search_file path contract regression) and move T0-6 status to doing.\n\nWhy:\n- Keep claim ledger, dependency board, and feature lifecycle synchronized with actual merged state.\n- Start next highest-priority, dependency-free P0 fix immediately after D7 closure.\n\nVerification:\n- ./scripts/ci/check_governance_evidence_truth.sh => passed\n- ./scripts/ci/check_governance_traceability.sh => passed\n- ./scripts/ci/check_design_doc_drift.sh => passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant