diff --git a/crates/xgen-cli/src/composition.rs b/crates/xgen-cli/src/composition.rs index b0a5e68..1e2f3ac 100644 --- a/crates/xgen-cli/src/composition.rs +++ b/crates/xgen-cli/src/composition.rs @@ -1488,6 +1488,21 @@ impl PlanMaterializer for LocalReadMaterializer { Self::Workspace(materializer) => materializer.materialize(request), } } + + fn planned_arguments( + &self, + step_id: &str, + capability: &CapabilityRef, + reference: &ReconstructableMaterialReference, + material_digest: &str, + ) -> Option { + match self { + Self::ExactFiles(_) => None, + Self::Workspace(materializer) => { + materializer.planned_arguments(step_id, capability, reference, material_digest) + } + } + } } struct LocalMaterializer { @@ -1526,6 +1541,25 @@ impl PlanMaterializer for LocalMaterializer { } self.filesystem.materialize(request) } + + fn planned_arguments( + &self, + step_id: &str, + capability: &CapabilityRef, + reference: &ReconstructableMaterialReference, + material_digest: &str, + ) -> Option { + if capability.capability_id == PROCESS_EXECUTE_CAPABILITY_ID { + return self.process.as_ref()?.planned_arguments( + step_id, + capability, + reference, + material_digest, + ); + } + self.filesystem + .planned_arguments(step_id, capability, reference, material_digest) + } } #[allow(clippy::too_many_arguments, clippy::too_many_lines)] diff --git a/crates/xgen-cli/src/material_catalog.rs b/crates/xgen-cli/src/material_catalog.rs index a5ae463..10fc290 100644 --- a/crates/xgen-cli/src/material_catalog.rs +++ b/crates/xgen-cli/src/material_catalog.rs @@ -387,6 +387,23 @@ impl PlanMaterializer for WorkspaceReadMaterializer { } self.catalog.persist_request(&request) } + + fn planned_arguments( + &self, + step_id: &str, + capability: &CapabilityRef, + reference: &ReconstructableMaterialReference, + material_digest: &str, + ) -> Option { + self.catalog + .workspace_arguments( + reference, + step_id, + material_digest, + &capability.capability_id, + ) + .ok() + } } pub(crate) struct WorkspaceReadMaterialProvider { @@ -509,6 +526,23 @@ impl PlanMaterializer for ProcessMaterializer { } self.catalog.persist_process_request(&request) } + + fn planned_arguments( + &self, + step_id: &str, + capability: &CapabilityRef, + reference: &ReconstructableMaterialReference, + material_digest: &str, + ) -> Option { + if capability.capability_id != PROCESS_EXECUTE_CAPABILITY_ID + || capability.contract_version != PROCESS_EXECUTE_CONTRACT_VERSION + { + return None; + } + self.catalog + .process_arguments(reference, step_id, material_digest) + .ok() + } } pub(crate) struct ProcessMaterialProvider { diff --git a/crates/xgen-cli/tests/interactive_repl.rs b/crates/xgen-cli/tests/interactive_repl.rs index 2210a53..de4ce92 100644 --- a/crates/xgen-cli/tests/interactive_repl.rs +++ b/crates/xgen-cli/tests/interactive_repl.rs @@ -502,6 +502,11 @@ fn one_invalid_response_is_retried_and_the_run_completes_without_resume() { .map(|_| server.requests.recv_timeout(TEST_TIMEOUT).unwrap()) .collect(); server.handle.join().unwrap(); + // ADR-0065: the finished read shows the arguments it ran with. + assert_eq!( + planning_context(&requests[1])["steps"][0]["arguments"]["path"], + "workspace:primary/README.md" + ); // ADR-0064: only the call after the discarded response learns why it was discarded. assert!( planning_context(&requests[1]) diff --git a/crates/xgen-runtime/src/agent_loop.rs b/crates/xgen-runtime/src/agent_loop.rs index 7a714a1..49d49d5 100644 --- a/crates/xgen-runtime/src/agent_loop.rs +++ b/crates/xgen-runtime/src/agent_loop.rs @@ -11,12 +11,13 @@ use xgen_policy::ResourceResolver; use xgen_workgraph::{ AcceptedPlanStep, AgentLoopBudget, CompletionCandidateState, CompletionOutputError, CompletionOutputRecord, ContinuationAction, DependencyBlockReason, ExpectedPlanningTurn, - FrontierAction, MAX_ACCEPTED_OBJECTIVE_BYTES, MAX_ACCEPTED_PLAN_EDGES, MAX_ACCEPTED_PLAN_STEPS, - ModelCallAbandonmentReason, ModelCallBudget, ModelCallRejectionReason, ModelCallReservation, - ModelCallSettlement, ModelCallStatus, ModelCallUnknownReason, PlannedExecutionProfile, - PlannedInvocationMaterialRecord, PlannedInvocationSpec, PlanningContractError, - ProposalRejectionCode, ReconstructableMaterialReference, RunEvent, RunEventBody, RunState, - StepStatus, ToolOutputRecord, WorkFrontier, dependency_release_block_reason, derive_frontier, + FrontierAction, InvocationMaterialRetention, MAX_ACCEPTED_OBJECTIVE_BYTES, + MAX_ACCEPTED_PLAN_EDGES, MAX_ACCEPTED_PLAN_STEPS, ModelCallAbandonmentReason, ModelCallBudget, + ModelCallRejectionReason, ModelCallReservation, ModelCallSettlement, ModelCallStatus, + ModelCallUnknownReason, PlannedExecutionProfile, PlannedInvocationMaterialRecord, + PlannedInvocationSpec, PlanningContractError, ProposalRejectionCode, + ReconstructableMaterialReference, RunEvent, RunEventBody, RunState, StepStatus, + ToolOutputRecord, WorkFrontier, dependency_release_block_reason, derive_frontier, receipt_releases_dependency, validate_completion_summary_candidate, }; @@ -37,6 +38,8 @@ const MAX_PLANNING_CATALOG_DEFINITIONS: usize = 1_024; const MAX_PLANNING_SOURCE_STEPS: usize = 4_096; const MAX_PLANNING_DEFINITION_BYTES: usize = 256 * 1024; const MAX_PLANNING_STEP_BYTES: usize = 64 * 1024; +/// Larger arguments, such as patch bodies, stay out of the planning context (ADR-0065). +const MAX_PLANNING_STEP_ARGUMENT_BYTES: usize = 2 * 1024; const MAX_PLANNING_SOURCE_BYTES: usize = 8 * 1024 * 1024; const MAX_PLANNING_VALUE_DEPTH: usize = 64; const MAX_PLANNING_VALUE_NODES: usize = 32_768; @@ -193,6 +196,10 @@ pub struct PlanningStepSummary { status: StepStatus, attempts: u32, capability: Option, + /// Normalized arguments of a finished Step, when its materializer can return them within + /// `MAX_PLANNING_STEP_ARGUMENT_BYTES` (ADR-0065). + #[serde(skip_serializing_if = "Option::is_none")] + arguments: Option, dependency_released: bool, /// Value-free failure classification of a `failed` Step (ADR-0054); absent otherwise and for /// failures recorded before classification existed. @@ -336,6 +343,7 @@ impl fmt::Debug for PlanningStepSummary { .field("status", &self.status) .field("attempts", &self.attempts) .field("capability", &self.capability) + .field("arguments", &self.arguments.as_ref().map(|_| "")) .field("dependency_released", &self.dependency_released) .field("failure_code", &self.failure_code) .field("failure_meaning", &self.failure_meaning) @@ -969,6 +977,21 @@ pub trait PlanMaterializer { &mut self, request: PlanMaterializationRequest<'_>, ) -> Result; + + /// Normalized arguments of a finished Step whose material this materializer persisted, shown + /// to later planner turns so they can tell which call produced an output (ADR-0065). + /// + /// Implementations must return the arguments only when `reference` reconstructs to exactly + /// this Step, capability, and material digest. `None` leaves the arguments out. + fn planned_arguments( + &self, + _step_id: &str, + _capability: &CapabilityRef, + _reference: &ReconstructableMaterialReference, + _material_digest: &str, + ) -> Option { + None + } } /// Why the loop deliberately yielded without selecting or committing more work. @@ -1380,11 +1403,13 @@ impl AgentLoop { if planning_snapshot.state != state { return Err(AgentLoopError::PlanningSnapshotMismatch); } + let step_arguments = finished_step_arguments(store, materializer, &state); let context = match build_context_ordered( &state, planning_snapshot.completed_tool_outputs(), planning_snapshot.planning_step_order(), planning_snapshot.completed_tool_output_order(), + &step_arguments, &frontier, capabilities, &self.planning_constraints, @@ -2310,6 +2335,7 @@ fn build_context( completed_tool_outputs, &planning_step_order, &completed_tool_output_order, + &BTreeMap::new(), frontier, registry, planning_constraints, @@ -2317,12 +2343,65 @@ fn build_context( ) } +/// Arguments of finished Steps, read back through the materializer that persisted them (ADR-0065). +/// +/// A Step is skipped when its material is missing, ephemeral, bound to another digest, or larger +/// than `MAX_PLANNING_STEP_ARGUMENT_BYTES`; the context then shows it without arguments. +fn finished_step_arguments( + store: &S, + materializer: &M, + state: &RunState, +) -> BTreeMap +where + S: RunStore, + M: PlanMaterializer, +{ + let mut arguments = BTreeMap::new(); + for step in state.steps.values() { + if !matches!(step.status, StepStatus::Completed | StepStatus::Failed) { + continue; + } + let (Some(intent), Some(invocation)) = (&step.intent, &step.planned_invocation) else { + continue; + }; + let Ok(Some(material)) = store.load_invocation_material(&intent.effect_id) else { + continue; + }; + let InvocationMaterialRetention::ReconstructableReference(reference) = material.retention() + else { + continue; + }; + if material.step_id() != step.step_id + || material.material_digest() != invocation.plan_input_digest() + { + continue; + } + let capability = CapabilityRef { + capability_id: invocation.capability_id().to_owned(), + contract_version: invocation.contract_version().to_owned(), + }; + let Some(value) = materializer.planned_arguments( + &step.step_id, + &capability, + reference, + material.material_digest(), + ) else { + continue; + }; + if canonical_size(&value).is_ok_and(|size| size <= MAX_PLANNING_STEP_ARGUMENT_BYTES) { + arguments.insert(step.step_id.clone(), value); + } + } + arguments +} + #[allow(clippy::too_many_arguments, clippy::too_many_lines)] fn build_context_ordered( state: &RunState, completed_tool_outputs: &BTreeMap, planning_step_order: &[String], completed_tool_output_order: &[String], + step_arguments: &BTreeMap, frontier: &WorkFrontier, registry: &CapabilityRegistry, planning_constraints: &[PlanningConstraint], @@ -2479,6 +2558,7 @@ fn build_context_ordered( capability_id: invocation.capability_id().to_owned(), contract_version: invocation.contract_version().to_owned(), }), + arguments: step_arguments.get(step_id).cloned(), dependency_released: receipt_releases_dependency(step), failure_code: step.failure_label(), failure_meaning: step.failure_meaning(), @@ -3536,6 +3616,7 @@ mod tests { &BTreeMap::new(), &["step-z".to_owned(), "step-a".to_owned()], &[], + &BTreeMap::new(), &frontier, &CapabilityRegistry::new(), &[], @@ -3558,6 +3639,7 @@ mod tests { &BTreeMap::new(), &["step-a".to_owned(), "step-a".to_owned()], &[], + &BTreeMap::new(), &frontier, &CapabilityRegistry::new(), &[], @@ -3571,6 +3653,7 @@ mod tests { &BTreeMap::new(), &["step-z".to_owned(), "step-a".to_owned()], &["step-a".to_owned()], + &BTreeMap::new(), &frontier, &CapabilityRegistry::new(), &[], @@ -3618,6 +3701,7 @@ mod tests { &BTreeMap::new(), &order, &[], + &BTreeMap::new(), &frontier, &CapabilityRegistry::new(), &[], @@ -3636,6 +3720,38 @@ mod tests { assert!(value["steps"][0].get("repeatMeaning").is_none()); } + #[test] + fn finished_step_arguments_follow_the_capability_of_their_step() { + // ADR-0065: the planner sees which call produced an output. + let state = state(vec![ + invoked(step("listed", StepStatus::Completed, Vec::new()), 'a'), + invoked(step("unlisted", StepStatus::Completed, Vec::new()), 'b'), + ]); + let frontier = derive_frontier(&state).expect("frontier should derive"); + let order = ["listed", "unlisted"].map(str::to_owned); + let arguments = BTreeMap::from([("listed".to_owned(), serde_json::json!({"path": "src"}))]); + let context = build_context_ordered( + &state, + &BTreeMap::new(), + &order, + &[], + &arguments, + &frontier, + &CapabilityRegistry::new(), + &[], + &test_budget(), + ) + .expect("context should fit"); + let value = serde_json::to_value(&context).expect("context should serialize"); + assert_eq!( + value["steps"][0]["arguments"], + serde_json::json!({"path": "src"}) + ); + assert!(value["steps"][1].get("arguments").is_none()); + let raw = serde_json::to_string(&context).expect("context should serialize"); + assert!(raw.find("\"capability\"") < raw.find("\"arguments\"")); + } + #[test] fn planning_context_wire_order_is_stable_first_and_volatile_last() { // ADR-0036: provider prefix caches reuse only a leading run of identical tokens, so the diff --git a/docs/adr/0064-planning-feedback.md b/docs/adr/0064-planning-feedback.md index d02f02c..f5c6c8c 100644 --- a/docs/adr/0064-planning-feedback.md +++ b/docs/adr/0064-planning-feedback.md @@ -57,4 +57,4 @@ ADR-0061은 거절된 응답을 2번까지 다시 묻지만 이유는 알려 주 해석: 이 결정으로 생긴 회귀는 보이지 않는다. 읽기 반복 거절은 짧은 루프를 끊지만, 측정에서 가장 긴 루프는 프로세스 실행 반복이었고 이 결정은 그것을 막지 않는다. 같은 명령 반복은 결과를 비교해야 판단할 수 있어 -다음 결정으로 남긴다. 거절 이유가 다음 응답을 고치는지는 표본이 작아 판단하지 않는다. +다음 결정으로 남긴다. 원인은 끝난 Step의 인자가 context에 없던 것이었다([ADR-0065](0065-finished-step-arguments.md)). 거절 이유가 다음 응답을 고치는지는 표본이 작아 판단하지 않는다. diff --git a/docs/adr/0065-finished-step-arguments.md b/docs/adr/0065-finished-step-arguments.md new file mode 100644 index 0000000..9e9eec7 --- /dev/null +++ b/docs/adr/0065-finished-step-arguments.md @@ -0,0 +1,73 @@ +# ADR-0065: 끝난 Step의 인자를 planning context에 넣기 + +- 상태: Accepted +- 기준일: 2026-10-27 +- 변경 분류: 범용 planning context 변경. 특정 모델·작업·업무 규칙 없음. +- 기존 경계: ADR-0036(prefix 순서), ADR-0064(반복 표시·읽기 반복 거절) +- protocol 변경: planning context의 Step에 선택 필드 `arguments`가 붙는다. 값이 없으면 직렬화하지 않는다. + profile version은 v3 그대로다. +- journal·SQLite schema 변경: 없음. 인자는 이미 저장된 material recipe에서 다시 읽는다. + +## 문제 + +ADR-0064 뒤에도 같은 명령을 결과를 받고도 다시 실행하며 96 turn을 다 쓰는 경우가 남았다. +`~/projects/app` 아래 dirty 저장소 찾기(T4)에서 첫 `bash` 실행이 정답(저장소 이름 목록)을 내놓았는데도 +모델은 같은 명령을 45번 다시 실행했다. + +요청과 응답을 기록하는 loopback proxy로 실제 context를 확인했다. toolOutputs에는 stdout이 그대로 있었지만 +steps에는 objective와 capability만 있고 어떤 인자로 실행했는지가 없었다. 모델이 완료를 거절한 이유를 적은 +응답이 원인을 보여 준다. "두 실행 모두 저장소 목록만 출력했고 커밋되지 않은 변경 여부를 확인하는 출력이 +없다." 모델은 stdout이 `git status`로 거른 결과인지 확인할 수 없어서 다시 실행했다. + +같은 context를 그대로 다시 보낸 재생 실험(deepseek flash, 지점 4곳 x 4회): + +| context | 원래 그대로 | 끝난 Step에 인자 추가 | +| --- | --- | --- | +| T4 2번째 call | 완료 4/4 | 완료 4/4 | +| T4 3번째 call | 완료 4/4 | 완료 4/4 | +| T4 6번째 call (반복 시작 뒤) | 계획 4/4 | 완료 4/4 | +| T4 11번째 call | 계획 4/4 | 완료 4/4 | +| T3 rename 중간 3곳 (조기 완료 확인) | 계획 9/9 | 계획 9/9 | + +system prompt에 "목표가 이미 충족되면 완료하라"는 규칙을 더하는 방법도 재생했지만 오히려 계획을 더 냈다. + +## 결정 + +1. `PlanMaterializer`에 기본 구현이 `None`인 `planned_arguments`를 둔다. material을 저장한 materializer가 + reference를 다시 열어 같은 Step·capability·material digest일 때만 정규화된 인자를 돌려준다. +2. runtime은 끝난(Completed·Failed) Step마다 store의 invocation material을 읽어 digest가 Step의 + `planInputDigest`와 같을 때만 인자를 묻는다. 정규화된 canonical 크기가 2 KiB를 넘는 인자(patch 본문, + 파일 내용)는 넣지 않는다. +3. 인자는 Step 요약의 `capability` 바로 뒤에 `arguments`로 들어간다. 끝난 Step의 인자는 바뀌지 않으므로 + prefix cache를 깨지 않는다. +4. CLI는 workspace(`xgeny.fs/*`)와 `xgeny.process/execute`를 구현한다. 정확한 파일 허용(`--allow-file`), + discovery, terminal, web search는 아직 돌려주지 않는다. + +인자는 정규화된 형태다(`workspace:primary/README.md`, `process:primary/executables/bash`). 이 형태는 +capability 입력으로도 받아들여지므로 모델이 그대로 다시 써도 거절되지 않는다(e2e로 확인). + +## 대안 + +- **system prompt에 완료 규칙 추가:** 재생에서 효과가 없었다. +- **모델이 쓴 원래 인자를 따로 저장:** journal이나 새 sidecar가 필요하다. 저장된 recipe로 충분하다. +- **process 출력에 실행한 명령을 넣기:** capability 출력 계약을 capability마다 바꿔야 한다. + +## 검증 + +- 단위: 인자는 map에 있는 Step에만 붙고 `capability` 뒤에 온다. +- e2e: 읽기가 끝난 다음 call의 context에 그 Step의 `arguments.path`가 들어간다. +- 실사용 (deepseek flash, `--trust`, main build와 비교): + +| 작업 | main | 이 변경 | +| --- | --- | --- | +| T4 dirty 저장소 (설계 사례) | 96 call 소진 2회, 3 call 완료 1회 | 2 call, 4 call 완료 | +| H5 `package.json` 있는 디렉터리 (검증 사례) | 96 call 소진 2/2 (178초, 185초) | 2 call 완료 2/2 (4초) | +| H3 디렉터리별 git 브랜치 (검증 사례) | 5 call 2/2 | 5 call 2/2 | +| H4 `async def`가 가장 많은 파일 (검증 사례) | 3 call 2/2 | 3 call 2/2 | +| T3 rename | `planner_invalid_response`로 멈춤 2/2 | 완료 1/2 (45 committed), 1/2는 읽기 반복 거절 3연속 | +| T1, T6 | | 통과 | +| T2 | | 숨은 검사 통과, 모델이 기존 테스트 하나를 깨뜨림 | + +H5는 T4와 같은 모양(밖의 디렉터리를 셸로 훑는 질문)에서 같은 반복이 났고 인자를 넣자 사라졌다. H3·H4는 +반복이 없던 작업이라 call 수가 같다. T3의 남은 실패는 모델 JSON의 괄호 오류와 patch 실패로, 이 결정과 무관하다. +T4·H5 답의 정확도(T4 16/24, worktree 누락)는 모델이 고른 명령의 문제로 남는다.