From c66a0c140a9433be4f54e14a03b72f4136da2a10 Mon Sep 17 00:00:00 2001 From: Son Seong Jun <166786347+SonAIengine@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:05:24 +0900 Subject: [PATCH] =?UTF-8?q?=EB=82=98=EC=A4=91=EC=97=90=20=EB=B0=94?= =?UTF-8?q?=EB=80=90=20=EC=9E=90=EC=9B=90=EC=9D=98=20=EA=B4=80=EC=B0=B0=20?= =?UTF-8?q?=EA=B2=B0=EA=B3=BC=EC=97=90=20supersededBy=20=ED=91=9C=EC=8B=9C?= =?UTF-8?q?=EB=A5=BC=20=EB=8B=AC=EC=95=84=20=EC=9D=B4=EB=AF=B8=20=EC=A0=81?= =?UTF-8?q?=EC=9A=A9=ED=95=9C=20patch=20=EC=9E=AC=EC=A0=81=EC=9A=A9?= =?UTF-8?q?=EC=9D=84=20=EC=A4=84=EC=98=80=EB=8B=A4=20(ADR-0069)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H6(필드 rename)의 patch_context_missing 271개가 전부 이미 적용한 patch의 재적용이었고, 모델은 patch 전에 한 검색·읽기 결과의 옛 줄을 oldText로 복사했다. read-only 관찰이 덮는 자원(resource selector로 정규화 인자에서 읽음)을 뒤에 완료된 변경 Step이 바꿨으면 그 toolOutputs 항목에 supersededBy와 고정 사실 문장을 붙인다. 출력은 그대로 둔다. - 기록 재현 42회: 재적용 제안 25 -> 11 (실패 문장만 바꾸면 22, no-op 성공 처리 24) - 출력을 숨기는 안(4/42)은 held-out H7에서 할 일 목록을 잃어 3/3 미완료로 회귀해 버림 - 실행: H6 재적용 0/0/0회(이전 0, 28회), held-out H7 3/3 완료(이전 2/2), 테스트 745개 통과 - H6는 같은 읽기 반복(temperature 0)으로 여전히 멈춰 완료율은 그대로, 별도 과제 Co-Authored-By: Claude Opus 5.5 --- crates/xgen-cli/tests/workspace_discovery.rs | 108 +++++++++ crates/xgen-runtime/src/agent_loop.rs | 242 ++++++++++++++++--- docs/adr/0069-superseded-observations.md | 71 ++++++ 3 files changed, 392 insertions(+), 29 deletions(-) create mode 100644 docs/adr/0069-superseded-observations.md diff --git a/crates/xgen-cli/tests/workspace_discovery.rs b/crates/xgen-cli/tests/workspace_discovery.rs index 5b51218..fe2f507 100644 --- a/crates/xgen-cli/tests/workspace_discovery.rs +++ b/crates/xgen-cli/tests/workspace_discovery.rs @@ -847,6 +847,114 @@ fn exact_patch_requires_write_approval_and_survives_process_resume() { assert!(output.get("edits").is_none()); } +#[test] +fn observations_of_a_file_changed_later_are_marked_for_the_planner() { + // ADR-0069: a read or search covering a file that a later completed patch changed is marked + // with that patch; unrelated and later observations are not. Every output stays. + let fixture = tempdir().expect("test directory should exist"); + let state_root = fixture.path().join("state"); + let workspace = fixture.path().join("workspace"); + fs::create_dir_all(workspace.join("src")).expect("workspace should create"); + let source = "pub fn answer() -> u32 {\n 41\n}\n"; + fs::write(workspace.join("src/lib.rs"), source).expect("source should write"); + fs::write(workspace.join("README.md"), "fixture workspace").expect("README should write"); + let server = SequentialServer::spawn_responses(vec![ + plan_response( + "read_source", + "Read the source", + "xgeny.fs/read-text", + &json!({"path": "src/lib.rs"}), + ), + plan_response( + "read_readme", + "Read the README", + "xgeny.fs/read-text", + &json!({"path": "README.md"}), + ), + plan_response( + "search_source", + "Find the answer", + "xgeny.fs/search-text", + &json!({"path": "src", "query": "41"}), + ), + plan_response( + "patch_source", + "Patch the answer", + "xgeny.fs/apply-patch", + &json!({ + "path": "src/lib.rs", + "expectedDigest": test_sha256_digest(source.as_bytes()), + "edits": [{"oldText": " 41\n", "newText": " 42\n"}] + }), + ), + plan_response( + "reread_source", + "Read the patched source", + "xgeny.fs/read-text", + &json!({"path": "src/lib.rs"}), + ), + completion_response(), + ]); + + let output = bounded_output(xgen(&state_root).args([ + "run", + "--workspace", + path_text(&workspace), + "--base-url", + &server.base_url, + "--model", + MODEL, + "--tokenizer", + TOKENIZER, + "--allow-dir", + ".", + "--allow-read", + "--allow-write", + "--allow-remote-model-egress", + "Update the answer and confirm it.", + ])) + .expect("run should finish"); + assert_eq!(output.status.code(), Some(0), "{}", stderr(&output)); + + let requests: Vec<_> = (0..6) + .map(|_| server.requests.recv_timeout(TEST_TIMEOUT).unwrap()) + .collect(); + server.handle.join().expect("provider server should finish"); + let before_patch = planning_context(&requests[3]); + assert!( + before_patch["toolOutputs"] + .as_array() + .unwrap() + .iter() + .all(|output| output.get("supersededBy").is_none()) + ); + let context = planning_context(&requests[5]); + let steps = context["steps"].as_array().unwrap(); + let patch_step = steps[3]["stepId"].as_str().unwrap(); + let outputs = context["toolOutputs"].as_array().unwrap(); + assert_eq!(outputs.len(), 5); + for stale in [&outputs[0], &outputs[2]] { + assert_eq!(stale["supersededBy"], patch_step, "{stale}"); + assert!( + stale["supersededMeaning"] + .as_str() + .unwrap() + .contains("may no longer match") + ); + } + assert_eq!(outputs[0]["output"]["content"], source); + assert!(outputs[2]["output"]["matches"].as_array().is_some()); + assert_eq!(outputs[1]["output"]["content"], "fixture workspace"); + assert_eq!(outputs[3]["output"]["changed"], true); + assert_eq!( + outputs[4]["output"]["content"], + "pub fn answer() -> u32 {\n 42\n}\n" + ); + for fresh in [&outputs[1], &outputs[3], &outputs[4]] { + assert!(fresh.get("supersededBy").is_none()); + } +} + #[test] fn failed_read_is_shown_to_the_planner_and_the_task_can_complete() { let fixture = tempdir().expect("test directory should exist"); diff --git a/crates/xgen-runtime/src/agent_loop.rs b/crates/xgen-runtime/src/agent_loop.rs index 4786721..714dbd6 100644 --- a/crates/xgen-runtime/src/agent_loop.rs +++ b/crates/xgen-runtime/src/agent_loop.rs @@ -39,6 +39,9 @@ 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). +/// Fixed `supersededMeaning` beside an observation whose output was withheld (ADR-0069). +const SUPERSEDED_OUTPUT_MEANING: &str = "A later completed Step changed a resource this observation \ + covered, so this output may no longer match the current state."; 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; @@ -236,6 +239,12 @@ pub struct PlanningToolOutput { receipt_digest: String, canonical_size_bytes: u64, output: Value, + /// The first later completed Step that changed a resource this observation covered (ADR-0069). + /// The output stays: unchanged parts of a broad observation remain the planner's only record. + #[serde(skip_serializing_if = "Option::is_none")] + superseded_by: Option, + #[serde(skip_serializing_if = "Option::is_none")] + superseded_meaning: Option<&'static str>, } impl PlanningToolOutput { @@ -273,6 +282,11 @@ impl PlanningToolOutput { pub const fn output(&self) -> &Value { &self.output } + + #[must_use] + pub fn superseded_by(&self) -> Option<&str> { + self.superseded_by.as_deref() + } } impl fmt::Debug for PlanningToolOutput { @@ -286,6 +300,8 @@ impl fmt::Debug for PlanningToolOutput { .field("receipt_digest", &self.receipt_digest) .field("canonical_size_bytes", &self.canonical_size_bytes) .field("output", &"") + .field("superseded_by", &self.superseded_by) + .field("superseded_meaning", &self.superseded_meaning) .finish() } } @@ -432,7 +448,8 @@ pub enum PlanningConstraintError { /// Field order is the wire order (ADR-0036): the catalog and goal are stable for a Run, `steps` /// and `tool_outputs` only grow at their tails between turns, and the per-call identifiers come -/// last, so a provider prefix cache can reuse the leading tokens across calls. The canonical +/// last, so a provider prefix cache can reuse the leading tokens across calls. A completed change +/// marks earlier outputs it superseded (ADR-0069), which shortens the reusable prefix once. The canonical /// `context_digest` is RFC 8785 and therefore independent of this order. #[derive(Clone, PartialEq, Eq, Serialize)] #[serde(rename_all = "camelCase")] @@ -1404,12 +1421,20 @@ impl AgentLoop { return Err(AgentLoopError::PlanningSnapshotMismatch); } let step_arguments = finished_step_arguments(store, materializer, capabilities, &state); + let superseded_outputs = superseded_observations( + store, + materializer, + capabilities, + &state, + planning_snapshot.completed_tool_output_order(), + ); 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, + &superseded_outputs, &frontier, capabilities, &self.planning_constraints, @@ -2246,6 +2271,7 @@ fn build_planning_tool_outputs( state: &RunState, completed_tool_outputs: &BTreeMap, completed_tool_output_order: &[String], + superseded_outputs: &BTreeMap, ) -> Result, ContextBuildError> { let expected_count = state .steps @@ -2311,6 +2337,10 @@ fn build_planning_tool_outputs( receipt_digest: receipt_digest.to_owned(), canonical_size_bytes: output.canonical_size_bytes(), output: output.output().clone(), + superseded_by: superseded_outputs.get(step_id).cloned(), + superseded_meaning: superseded_outputs + .contains_key(step_id) + .then_some(SUPERSEDED_OUTPUT_MEANING), }); } if seen.len() != expected_count { @@ -2336,6 +2366,7 @@ fn build_context( &planning_step_order, &completed_tool_output_order, &BTreeMap::new(), + &BTreeMap::new(), frontier, registry, planning_constraints, @@ -2364,44 +2395,158 @@ where if !matches!(step.status, StepStatus::Completed | StepStatus::Failed) { continue; } - let (Some(intent), Some(invocation)) = (&step.intent, &step.planned_invocation) else { + let Some(capability) = planned_capability(step) else { continue; }; - let capability = CapabilityRef { - capability_id: invocation.capability_id().to_owned(), - contract_version: invocation.contract_version().to_owned(), - }; - let non_idempotent = registry - .definition(&capability) - .is_some_and(|definition| definition.spec.effect.class == EffectClass::NonIdempotent); - if !non_idempotent { + if effect_class(registry, &capability) != Some(EffectClass::NonIdempotent) { continue; } - let Ok(Some(material)) = store.load_invocation_material(&intent.effect_id) else { + let Some(value) = normalized_step_arguments(store, materializer, step, &capability) else { continue; }; - let InvocationMaterialRetention::ReconstructableReference(reference) = material.retention() - else { - continue; + if canonical_size(&value).is_ok_and(|size| size <= MAX_PLANNING_STEP_ARGUMENT_BYTES) { + arguments.insert(step.step_id.clone(), value); + } + } + arguments +} + +fn planned_capability(step: &xgen_workgraph::StepState) -> Option { + step.planned_invocation + .as_ref() + .map(|invocation| CapabilityRef { + capability_id: invocation.capability_id().to_owned(), + contract_version: invocation.contract_version().to_owned(), + }) +} + +fn effect_class(registry: &CapabilityRegistry, capability: &CapabilityRef) -> Option { + registry + .definition(capability) + .map(|definition| definition.spec.effect.class) +} + +/// Normalized arguments of a Step whose material reconstructs to exactly this Step (ADR-0065). +fn normalized_step_arguments( + store: &S, + materializer: &M, + step: &xgen_workgraph::StepState, + capability: &CapabilityRef, +) -> Option +where + S: RunStore, + M: PlanMaterializer, +{ + let (intent, invocation) = (step.intent.as_ref()?, step.planned_invocation.as_ref()?); + let material = store.load_invocation_material(&intent.effect_id).ok()??; + let InvocationMaterialRetention::ReconstructableReference(reference) = material.retention() + else { + return None; + }; + if material.step_id() != step.step_id + || material.material_digest() != invocation.plan_input_digest() + { + return None; + } + materializer.planned_arguments( + &step.step_id, + capability, + reference, + material.material_digest(), + ) +} + +/// Resources a Step selected through its capability's resource selectors, read from its +/// normalized arguments. +fn selected_resources( + registry: &CapabilityRegistry, + capability: &CapabilityRef, + arguments: &Value, +) -> Vec { + registry + .definition(capability) + .map(|definition| { + definition + .spec + .effect + .resource_selectors + .iter() + .filter_map(|selector| arguments.pointer(&selector.argument_pointer)) + .filter_map(Value::as_str) + .map(str::to_owned) + .collect() + }) + .unwrap_or_default() +} + +/// Whether `changed` is `observed` or lies under it in a `/`-separated resource hierarchy. +fn resource_covers(observed: &str, changed: &str) -> bool { + changed == observed + || changed + .strip_prefix(observed) + .is_some_and(|rest| rest.starts_with('/') || observed.ends_with('/')) +} + +/// Read-only observations whose resources a later completed Step changed, keyed by the observing Step and +/// mapped to the first Step that changed one of its resources (ADR-0069). +/// +/// Completion order is receipt chronology, so an observation completed after the change is never +/// superseded by it. A Step whose arguments or resources are unknown supersedes nothing. +fn superseded_observations( + store: &S, + materializer: &M, + registry: &CapabilityRegistry, + state: &RunState, + completed_tool_output_order: &[String], +) -> BTreeMap +where + S: RunStore, + M: PlanMaterializer, +{ + let classified = completed_tool_output_order + .iter() + .filter_map(|step_id| { + let step = state.steps.get(step_id)?; + let capability = planned_capability(step)?; + let class = effect_class(registry, &capability)?; + Some((step, capability, class)) + }) + .collect::>(); + let Some(last_change) = classified + .iter() + .rposition(|(_, _, class)| *class != EffectClass::ReadOnly) + else { + return BTreeMap::new(); + }; + let mut observations: Vec<(&str, Vec)> = Vec::new(); + let mut superseded = BTreeMap::new(); + for (step, capability, class) in &classified[..=last_change] { + let resources = || { + normalized_step_arguments(store, materializer, step, capability) + .map(|arguments| selected_resources(registry, capability, &arguments)) + .unwrap_or_default() }; - if material.step_id() != step.step_id - || material.material_digest() != invocation.plan_input_digest() - { + if *class == EffectClass::ReadOnly { + observations.push((step.step_id.as_str(), resources())); continue; } - let Some(value) = materializer.planned_arguments( - &step.step_id, - &capability, - reference, - material.material_digest(), - ) else { + if observations.is_empty() { continue; - }; - if canonical_size(&value).is_ok_and(|size| size <= MAX_PLANNING_STEP_ARGUMENT_BYTES) { - arguments.insert(step.step_id.clone(), value); } + let changed = resources(); + observations.retain(|(observer, observed)| { + let covered = observed.iter().any(|observed| { + changed + .iter() + .any(|changed| resource_covers(observed, changed)) + }); + if covered { + superseded.insert((*observer).to_owned(), step.step_id.clone()); + } + !covered + }); } - arguments + superseded } #[allow(clippy::too_many_arguments, clippy::too_many_lines)] @@ -2411,6 +2556,7 @@ fn build_context_ordered( planning_step_order: &[String], completed_tool_output_order: &[String], step_arguments: &BTreeMap, + superseded_outputs: &BTreeMap, frontier: &WorkFrontier, registry: &CapabilityRegistry, planning_constraints: &[PlanningConstraint], @@ -2443,8 +2589,12 @@ fn build_context_ordered( .filter(|size| *size <= MAX_PLANNING_SOURCE_BYTES) .ok_or(ContextBuildError::InputLimitExceeded) })?; - let tool_outputs = - build_planning_tool_outputs(state, completed_tool_outputs, completed_tool_output_order)?; + let tool_outputs = build_planning_tool_outputs( + state, + completed_tool_outputs, + completed_tool_output_order, + superseded_outputs, + )?; source_bytes = tool_outputs .iter() .try_fold(source_bytes, |total, output| { @@ -3626,6 +3776,7 @@ mod tests { &["step-z".to_owned(), "step-a".to_owned()], &[], &BTreeMap::new(), + &BTreeMap::new(), &frontier, &CapabilityRegistry::new(), &[], @@ -3649,6 +3800,7 @@ mod tests { &["step-a".to_owned(), "step-a".to_owned()], &[], &BTreeMap::new(), + &BTreeMap::new(), &frontier, &CapabilityRegistry::new(), &[], @@ -3663,6 +3815,7 @@ mod tests { &["step-z".to_owned(), "step-a".to_owned()], &["step-a".to_owned()], &BTreeMap::new(), + &BTreeMap::new(), &frontier, &CapabilityRegistry::new(), &[], @@ -3711,6 +3864,7 @@ mod tests { &order, &[], &BTreeMap::new(), + &BTreeMap::new(), &frontier, &CapabilityRegistry::new(), &[], @@ -3729,6 +3883,35 @@ mod tests { assert!(value["steps"][0].get("repeatMeaning").is_none()); } + #[test] + fn a_change_covers_the_same_resource_or_one_below_it() { + // ADR-0069: hierarchy is `/`-separated; other resources only match exactly. + assert!(resource_covers( + "workspace:primary/src/lib.rs", + "workspace:primary/src/lib.rs" + )); + assert!(resource_covers( + "workspace:primary/src", + "workspace:primary/src/lib.rs" + )); + assert!(resource_covers( + "workspace:primary", + "workspace:primary/README.md" + )); + assert!(!resource_covers( + "workspace:primary/src", + "workspace:primary/srcs/lib.rs" + )); + assert!(!resource_covers( + "workspace:primary/src/lib.rs", + "workspace:primary/src" + )); + assert!(!resource_covers( + "workspace:primary/a.md", + "workspace:primary/b.md" + )); + } + #[test] fn finished_step_arguments_follow_the_capability_of_their_step() { // ADR-0065: the planner sees which call produced an output. @@ -3745,6 +3928,7 @@ mod tests { &order, &[], &arguments, + &BTreeMap::new(), &frontier, &CapabilityRegistry::new(), &[], diff --git a/docs/adr/0069-superseded-observations.md b/docs/adr/0069-superseded-observations.md new file mode 100644 index 0000000..f396585 --- /dev/null +++ b/docs/adr/0069-superseded-observations.md @@ -0,0 +1,71 @@ +# ADR-0069: 나중에 바뀐 자원의 관찰 결과에 표시 달기 + +- 상태: Accepted +- 기준일: 2026-10-27 +- 변경 분류: 범용 planning context. 특정 작업·언어·모델 규칙 없음. +- protocol·journal 변경: 없음. planning context v3의 `toolOutputs` 항목에 선택 필드 두 개가 붙는다. + +## 문제 + +여러 파일을 고치는 rename 작업에서 모델이 이미 적용한 patch를 계속 다시 제안해 `patch_context_missing`으로 +실패하고 예산을 다 썼다. 실패한 patch를 원본에 순서대로 다시 적용해 분류하면 H6(필드 rename)의 +`patch_context_missing` 271개 전부가 "oldText는 없고 newText는 이미 있는" 재적용이었다. 13번 실행 중 8번이 +재적용 15~55회를 냈다. + +모델이 보는 기록을 보면, patch 전에 한 검색 결과(`devto.py:199 likes=...`)와 읽기 결과가 그대로 남아 있고 모델은 +그 옛 줄을 oldText로 복사했다. 같은 파일을 patch 뒤에 다시 읽은 결과가 있어도 옛 결과를 썼다. + +기록 재현(H6 실행 7개에서 재적용 직후 14개 지점, 지점당 3회, deepseek flash)으로 원인을 확인했다. + +| 맥락 변경 | 재적용 제안 | +| --- | --- | +| 그대로 | 25/42 | +| 실패 이유 문장을 "이미 적용된 것 같다"로 바꿈 | 22/42 | +| 이미 적용된 patch를 변화 없는 성공으로 처리 | 24/42 | +| patch 전 검색 결과만 뺌 | 15/42 | +| 바뀐 파일의 옛 읽기·검색 결과를 뺌 | 2/42 | +| 같은 결과에 출력 대신 "나중에 바뀜" 표시 | 4/42 | +| 출력은 두고 "나중에 바뀜" 표시만 | 11/42 | + +## 결정 + +planning context를 만들 때, read-only Step의 관찰 결과가 덮는 자원을 그 뒤에 완료된 변경 Step(effect class가 +read-only가 아닌 Step)이 바꿨으면 그 `toolOutputs` 항목에 두 필드를 붙인다. + +- `supersededBy`: 그 자원을 처음 바꾼 완료 Step의 id +- `supersededMeaning`: 고정 사실 문장 "A later completed Step changed a resource this observation covered, so + this output may no longer match the current state." + +출력은 그대로 둔다. + +- 자원은 capability 정의의 resource selector(`argumentPointer`)로 정규화된 인자(ADR-0065의 material 복원)에서 읽는다. + 도메인별 규칙은 없다. +- 덮는다는 판단은 같은 자원이거나 `/`로 나뉜 계층에서 그 아래인 경우다. 계층이 없는 자원은 같을 때만 맞는다. +- 순서는 receipt 완료 순서다. 변경 뒤에 끝난 관찰에는 표시가 붙지 않는다. +- 인자나 자원을 모르는 Step(복원 실패, process 실행 등)은 아무것도 덮지 않는다. `bash -c 'sed -i ...'`처럼 process로 + 바꾼 파일은 감지하지 못한다. +- 앞쪽 항목이 바뀌므로 provider prefix cache가 그 지점부터 한 번 다시 계산된다(ADR-0036). + +### 출력을 숨기지 않은 이유 + +처음에는 표를 근거로 출력을 숨겼다(4/42). held-out H7(다른 필드 rename)에서 회귀가 나왔다. + +- 이전 빌드는 2/2 모두 남은 사용처 0개로 끝났다. +- 숨김 빌드는 3/3 모두 8~9곳을 남기고 완료를 선언했다. + +모델은 처음 한 패키지 검색 결과를 할 일 목록으로 쓰는데, 첫 patch 뒤 그 결과가 숨겨지자 workspace 전체를 다시 +검색했다. 그 결과는 `.venv` 매치로 잘려 일부만 보였다. 넓은 관찰은 바뀌지 않은 부분의 유일한 기록이라서 숨기면 안 된다. + +## 실행 측정 (deepseek flash, --trust) + +| 작업 | 빌드 | 결과 | +| --- | --- | --- | +| H6 (설계 사례) | 이전 | 재적용 0, 28회 / 남은 사용처 10, 14 | +| H6 | 표시 | 재적용 0, 0, 0회 / 3회 모두 같은 읽기 9번 반복(`repeated_finished_action`)으로 멈춤, 남은 사용처 12~20 | +| H7 (held-out) | 이전 | 2/2 완료, 남은 사용처 0 | +| H7 | 표시 | 3/3 완료, 남은 사용처 0, 32~38 call | +| T2 (held-out) | 표시 | 1/2 hidden test 통과, 1회는 `material_rejected`(이전 빌드에도 8번 중 2번) | + +재적용 루프는 사라졌다. 하지만 H6 완료율은 아직 오르지 않았다. 남은 원인은 temperature 0에서 같은 읽기를 +계속 고르는 것이다. 멈춘 지점 4곳을 재현하면 24번 중 24번이 같은 읽기를 골랐고, 이 변경 전 빌드에도 +같은 방식으로 멈춘 실행이 T3·H6에서 8번 있었다. 이건 별도로 다룬다.