diff --git a/crates/xgen-provider-openai/src/lib.rs b/crates/xgen-provider-openai/src/lib.rs index df9cea1..96084d4 100644 --- a/crates/xgen-provider-openai/src/lib.rs +++ b/crates/xgen-provider-openai/src/lib.rs @@ -1571,7 +1571,14 @@ fn decode_chat_response_with_codec( if content.len() > max_proposal_bytes { return Err(PlannerPortFailure::InvalidResponse); } - let proposal_value = parse_unique_json(content.as_bytes(), max_json_depth)?; + let proposal_value = + parse_unique_json(content.as_bytes(), max_json_depth).or_else(|error| { + // ADR-0068: a model that drops a closer inside nested arguments gets one rebalance. + rebalance_json_closers(content) + .filter(|repaired| repaired.len() <= max_proposal_bytes) + .and_then(|repaired| parse_unique_json(repaired.as_bytes(), max_json_depth).ok()) + .ok_or(error) + })?; let proposal: ProposalDocument = serde_json::from_value(proposal_value).map_err(|_| PlannerPortFailure::InvalidResponse)?; if proposal.format_version != 1 { @@ -1850,6 +1857,53 @@ fn parse_unique_json(body: &[u8], max_depth: usize) -> Result Option { + let mut repaired = String::with_capacity(text.len() + 8); + let mut open = Vec::new(); + let mut in_string = false; + let mut escaped = false; + let mut inserted = false; + for character in text.chars() { + if in_string { + if escaped { + escaped = false; + } else if character == '\\' { + escaped = true; + } else if character == '"' { + in_string = false; + } + repaired.push(character); + continue; + } + match character { + '"' => in_string = true, + '{' => open.push('}'), + '[' => open.push(']'), + '}' | ']' => { + if !open.contains(&character) { + return None; + } + while let Some(closer) = open.pop() { + if closer == character { + break; + } + repaired.push(closer); + inserted = true; + } + } + _ => {} + } + repaired.push(character); + } + (inserted && !in_string && open.is_empty()).then_some(repaired) +} + fn json_depth_within(body: &[u8], max_depth: usize) -> bool { let mut depth = 0_usize; let mut in_string = false; @@ -3097,6 +3151,47 @@ mod tests { ); } + #[test] + fn a_dropped_closer_in_nested_arguments_is_rebalanced() { + // ADR-0068: the shape observed in patch proposals, with brackets inside a string. + let step = r#"{"key":"patch","objective":"Rename","dependsOn":[],"capability":{"capabilityId":"xgeny.fs/apply-patch","contractVersion":"2.0.0"},"arguments":{"path":"a.py","edits":[{"oldText":"x[\"id\"]}","newText":"y[\"id\"]}"}]}"#; + let correct = + format!(r#"{{"formatVersion":1,"kind":"plan","steps":[{step}}}],"summary":""}}"#); + let dropped = + format!(r#"{{"formatVersion":1,"kind":"plan","steps":[{step}],"summary":""}}"#); + let expected = decode_chat_response(&response(&correct, "stop"), MODEL, 256 * 1024, 64) + .expect("the correct plan should decode"); + assert_eq!( + decode_chat_response(&response(&dropped, "stop"), MODEL, 256 * 1024, 64), + Ok(expected) + ); + assert_eq!(rebalance_json_closers(&correct), None); + } + + #[test] + fn closers_are_not_invented_for_truncated_or_unmatched_json() { + for content in [ + // still open at the end: truncation is never completed + r#"{"formatVersion":1,"kind":"plan","steps":[{"key":"a""#, + // a closer with no matching open container + r#"{"formatVersion":1}]"#, + // the extra closer after a full object + r#"{"steps":[{"a":1}]}}"#, + ] { + assert_eq!(rebalance_json_closers(content), None, "{content}"); + assert_eq!( + decode_chat_response(&response(content, "stop"), MODEL, 256 * 1024, 64), + Err(PlannerPortFailure::InvalidResponse) + ); + } + // brackets inside strings never count + assert_eq!(rebalance_json_closers(r#"{"a":"]}"}"#), None); + assert_eq!( + rebalance_json_closers(r#"{"a":[{"b":"]"]}"#).as_deref(), + Some(r#"{"a":[{"b":"]"}]}"#) + ); + } + #[test] fn strict_codec_requires_exactly_one_normal_assistant_choice() { let no_choices = serde_json::to_vec(&json!({"model": MODEL, "choices": []})).unwrap(); diff --git a/docs/adr/0068-json-closer-rebalance.md b/docs/adr/0068-json-closer-rebalance.md new file mode 100644 index 0000000..f4b586e --- /dev/null +++ b/docs/adr/0068-json-closer-rebalance.md @@ -0,0 +1,54 @@ +# ADR-0068: 모델 JSON의 어긋난 닫는 괄호 한 번 바로잡기 + +- 상태: Accepted +- 기준일: 2026-10-27 +- 변경 분류: 범용 provider 응답 해석. 특정 모델·작업 규칙 없음. +- protocol·journal 변경: 없음. 원래 응답 body와 그 digest는 그대로 기록된다. + +## 문제 + +긴 patch를 제안하는 응답이 JSON으로 파싱되지 않아 `planner_invalid_response`로 거절되는 일이 잦았다. +loopback proxy로 응답 원문을 모아 보니 deepseek flash 응답 2,205개 중 53개가 파싱에 실패했다. + +| 작업 | 응답 | 파싱 실패 | +| --- | --- | --- | +| T3 rename (patch가 2~4 KB) | 857 | 51 | +| T2 기능 추가 | 34 | 2 | +| H6 rename (patch 대부분 1 KB 미만) | 782 | 0 | +| 그 밖의 6개 작업 | 532 | 0 | + +53개 모두 오류가 `Expecting ',' delimiter`였고 위치는 끝부분이다. `steps[].arguments.edits[]`처럼 다섯 겹으로 +중첩된 객체를 닫을 때 `}` 하나를 빠뜨렸다(`}]}}]`를 `}]}]`로 씀). ADR-0061 재시도와 ADR-0064의 거절 이유를 +받아도 같은 긴 patch에서 다시 틀려 T3 Run이 이 거절 3연속으로 여러 번 멈췄다. + +## 결정 + +provider는 `content`가 JSON으로 파싱되지 않을 때 한 번만 괄호를 다시 맞춘다. + +1. 문자열 밖에서 닫는 괄호가 가장 안쪽의 열린 괄호와 짝이 맞지 않지만 더 바깥의 열린 괄호와는 맞으면, + 그 사이에 열린 괄호들의 닫는 괄호를 그 자리에 넣는다. +2. 짝이 없는 닫는 괄호가 있거나, 끝에 열린 괄호가 남거나(잘린 출력), 고칠 것이 없으면 바로잡지 않는다. + 잘린 출력은 지금처럼 `OutputTruncated`다. +3. 바로잡은 텍스트는 원래와 같은 엄격한 parser(중복 키 거부, 깊이 제한, 크기 제한)와 스키마 검사를 다시 + 통과해야 한다. 실패하면 원래 오류(`InvalidResponse`)를 그대로 낸다. + +제안은 처음부터 신뢰하지 않는 입력이고, 바로잡은 뒤에도 Core의 모든 검증(capability, 스키마, scope, 승인)을 +거친다. 괄호를 넣는 위치는 모델이 바깥 괄호를 닫은 자리라서, 안쪽 객체가 거기서 끝났다는 것 말고는 새로운 +의미를 만들지 않는다. + +## 대안 + +- **재시도만:** 이미 하고 있다. 같은 긴 patch에서 같은 실수를 반복한다. +- **provider의 엄격한 JSON schema 출력:** deepseek의 `json_object` 모드는 구조를 보장하지 않는다. 다른 provider는 + `json_schema`를 쓸 수 있고 그 경우 이 경로를 타지 않는다. +- **일반 JSON repair 라이브러리:** 따옴표·쉼표 보정까지 하면 의미를 바꿀 수 있다. 관찰된 오류는 닫는 괄호뿐이다. + +## 검증 + +- 단위: 관찰된 모양(문자열 안 괄호 포함)이 올바른 plan과 같은 결과로 해석된다. 잘린 JSON, 짝 없는 닫는 괄호, + 남는 닫는 괄호는 바로잡지 않는다. +- 기록 재현: 파싱 실패 53개 중 48개가 바로잡힌다(T3 46/51, T2 2/2). 나머지 5개는 `]}`의 순서를 바꾼 경우라 + 넣기만으로는 고칠 수 없어 그대로 거절된다. +- 실행 측정(바로잡기 포함 빌드, deepseek flash): T3 2회에서 응답 90개 중 4개가 괄호가 어긋났고 4개 모두 + 바로잡혀 진행됐다. `planner_invalid_response` 0회, 1회는 rename 완료(테스트 47 passed), 1회는 rename 일부만 + 끝내고 완료를 선언했다(별도 문제). held-out T2 2회는 어긋난 응답 없이 둘 다 hidden test 통과.