Skip to content

Commit 4a6444e

Browse files
committed
Fix compaction data loss, sideways photos, and dropped stream-error text
1 parent ab00d2e commit 4a6444e

9 files changed

Lines changed: 222 additions & 34 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ Gitignored scratchpad for helper files the user asks to be created there — typ
457457
- `cargo clippy --package sofos --target x86_64-pc-windows-gnu --all-targets`
458458
- Before finishing, review the change for bugs and corner cases.
459459
- Use international English. Avoid regional idioms (whether American or British), clever shorthand, and compressed phrases; prefer wording that a non-native English reader can understand on the first read. This applies to chat replies, commit messages, code comments, documentation, and error messages.
460-
- After you finish cross-checking against the Non-Negotiable Rules and fixing the code, if needed, do another pass for bugs and regressions.
460+
- After you finish cross-checking against the Non-Negotiable Rules and fixing the code, do another pass for bugs and regressions.
461461
- After each final modification, provide a clear, human-readable one-line commit message.
462462

463463
---

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ All notable changes to Sofos are documented in this file.
44

55
## [Unreleased]
66

7+
### Fixed
8+
9+
- **A failed or interrupted compaction no longer drops part of the conversation.** Compaction shortens older tool output before summarising; if the summary step then failed or was cancelled, the shortened output used to be saved anyway. The conversation is now kept intact unless compaction finishes cleanly.
10+
- **Photos are no longer sent to the model rotated sideways.** Images that get resized now keep the upright orientation recorded by the camera.
11+
- **A streamed reply cut short by a provider error keeps the text shown so far.** That text is carried into the error and the conversation, so a retry still has the context.
12+
713
## [0.3.2] - 2026-05-19
814

915
### Added

STRUCTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,7 @@ Rules:
10101010

10111011
It contains:
10121012

1013-
- decode plus optional resize (long side fits within 2048 pixels) before the bytes reach the model;
1013+
- decode, applying any orientation the photo's metadata records, plus optional resize (long side fits within 2048 pixels) before the bytes reach the model;
10141014
- byte-level format detection: PNG, JPEG, GIF, and WebP pass through unchanged when small enough; other decodable formats (e.g. BMP) are re-encoded as PNG;
10151015
- base64 encoding and media-type assignment;
10161016
- the 20 MB per-file size cap on the raw bytes;

src/api/anthropic/mod.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,35 @@ mod tests {
407407
);
408408
}
409409

410+
#[tokio::test]
411+
async fn error_event_keeps_partial_reply_text() {
412+
// A mid-stream error must keep the reply text already
413+
// streamed to the screen so the next turn is not blind to it.
414+
let events = vec![
415+
json!({"type": "message_start", "message": {"id": "msg_e", "model": "claude-sonnet-4-6", "usage": {"input_tokens": 1}}}),
416+
json!({"type": "content_block_start", "index": 0, "content_block": {"type": "text", "text": ""}}),
417+
json!({"type": "content_block_delta", "index": 0, "delta": {"type": "text_delta", "text": "Here is the fix: "}}),
418+
json!({"type": "content_block_delta", "index": 0, "delta": {"type": "text_delta", "text": "rename the field"}}),
419+
json!({"type": "error", "error": {"message": "overloaded"}}),
420+
];
421+
422+
let stream = sse_stream_from_events(events);
423+
let err = parse_stream(stream, |_| {}, |_| {}, flag())
424+
.await
425+
.expect_err("error event must surface as error");
426+
let crate::error::SofosError::Api(msg) = &err else {
427+
panic!("expected SofosError::Api, got: {err:?}");
428+
};
429+
assert!(
430+
msg.contains("overloaded"),
431+
"keeps the provider message: {msg}"
432+
);
433+
assert!(
434+
msg.contains("Here is the fix: rename the field"),
435+
"must carry the partial reply text: {msg}"
436+
);
437+
}
438+
410439
#[tokio::test]
411440
async fn multibyte_codepoint_split_across_chunks_is_preserved() {
412441
// Reproduces the pre-fix corruption: a UTF-8 codepoint

src/api/anthropic/stream.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ fn saturate_u32(n: u64) -> u32 {
4242
u32::try_from(n).unwrap_or(u32::MAX)
4343
}
4444

45+
/// Cap on streamed reply text folded into a mid-stream error, so the
46+
/// error and the saved session stay bounded.
47+
const MAX_PARTIAL_REPLY_BYTES: usize = 2000;
48+
4549
impl AnthropicClient {
4650
pub async fn create_message_streaming<FText, FThink>(
4751
&self,
@@ -405,7 +409,35 @@ where
405409
.and_then(|e| e.get("message"))
406410
.and_then(|m| m.as_str())
407411
.unwrap_or("Unknown streaming error");
408-
return Err(SofosError::Api(format!("Streaming error: {}", error_msg)));
412+
// Returning here drops the accumulated content_blocks,
413+
// so keep the streamed text in the error for the next turn.
414+
let mut partial: String = content_blocks
415+
.iter()
416+
.filter_map(|b| match b {
417+
ContentBlock::Text { text } => Some(text.as_str()),
418+
_ => None,
419+
})
420+
.collect();
421+
if current_block_type == Some(StreamBlockKind::Text) {
422+
partial.push_str(&current_text);
423+
}
424+
let partial = partial.trim();
425+
let context = if partial.is_empty() {
426+
String::new()
427+
} else {
428+
let cutoff =
429+
utils::truncate_at_char_boundary(partial, MAX_PARTIAL_REPLY_BYTES);
430+
let ellipsis = if cutoff < partial.len() { "…" } else { "" };
431+
format!(
432+
" (partial reply before the error: {}{})",
433+
&partial[..cutoff],
434+
ellipsis
435+
)
436+
};
437+
return Err(SofosError::Api(format!(
438+
"Streaming error: {}{}",
439+
error_msg, context
440+
)));
409441
}
410442
_ => {}
411443
}

src/repl/compaction.rs

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,27 @@ impl Repl {
3030
return Ok(false);
3131
}
3232

33-
self.session_state
34-
.conversation
35-
.truncate_tool_results(split_point);
36-
37-
if !force && !self.session_state.conversation.needs_compaction() {
38-
let tokens_after = self.session_state.conversation.estimate_total_tokens();
39-
println!(
40-
"\n{} {} -> {} tokens (tool results truncated)\n",
41-
"Compacted:".bright_green(),
42-
tokens_before,
43-
tokens_after
44-
);
45-
return Ok(true);
33+
// Truncate a clone first: commit the truncation only when it
34+
// alone frees enough tokens, so a failed or interrupted phase 2
35+
// never persists half-shortened tool results.
36+
if !force {
37+
let mut truncated = self.session_state.conversation.clone();
38+
truncated.truncate_tool_results(split_point);
39+
if !truncated.needs_compaction() {
40+
let tokens_after = truncated.estimate_total_tokens();
41+
self.session_state.conversation = truncated;
42+
println!(
43+
"\n{} {} -> {} tokens (tool results truncated)\n",
44+
"Compacted:".bright_green(),
45+
tokens_before,
46+
tokens_after
47+
);
48+
return Ok(true);
49+
}
4650
}
4751

48-
// Phase 2: Summarize older messages via the LLM
52+
// Phase 2: Summarize older messages via the LLM, leaving the
53+
// history un-truncated so a failed summary trims whole messages.
4954
let older_messages: Vec<_> =
5055
self.session_state.conversation.messages()[..split_point].to_vec();
5156
let serialized = ConversationHistory::serialize_messages_for_summary(&older_messages);

src/repl/conversation/mod.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,56 @@ mod tests {
586586
}
587587
}
588588

589+
#[test]
590+
fn truncate_tool_results_on_clone_leaves_original_intact() {
591+
// compact_conversation truncates a clone to test whether
592+
// truncation alone frees enough tokens; the original must stay
593+
// byte-identical so a failed summary can fall back to it.
594+
let mut history = ConversationHistory::new();
595+
history.config.tool_result_truncate_threshold = 100;
596+
let original_text = "x".repeat(500);
597+
history.messages.push(Message::user_with_tool_results(vec![
598+
MessageContentBlock::ToolResult {
599+
tool_use_id: "id1".to_string(),
600+
content: original_text.clone(),
601+
cache_control: None,
602+
},
603+
]));
604+
605+
let mut probe = history.clone();
606+
probe.truncate_tool_results(1);
607+
608+
let crate::api::MessageContent::Blocks { content } = &probe.messages()[0].content else {
609+
panic!("expected Blocks");
610+
};
611+
let MessageContentBlock::ToolResult {
612+
content: probe_text,
613+
..
614+
} = &content[0]
615+
else {
616+
panic!("expected ToolResult");
617+
};
618+
assert!(
619+
probe_text.contains("truncated"),
620+
"the clone must be truncated"
621+
);
622+
623+
let crate::api::MessageContent::Blocks { content } = &history.messages()[0].content else {
624+
panic!("expected Blocks");
625+
};
626+
let MessageContentBlock::ToolResult {
627+
content: original_after,
628+
..
629+
} = &content[0]
630+
else {
631+
panic!("expected ToolResult");
632+
};
633+
assert_eq!(
634+
original_after, &original_text,
635+
"truncating the clone must not touch the original conversation"
636+
);
637+
}
638+
589639
#[test]
590640
fn test_replace_with_summary() {
591641
let mut history = ConversationHistory::new();

src/tools/image.rs

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ use crate::error::{Result, ResultExt, SofosError};
22
use crate::tools::permissions::{CommandPermission, PermissionManager};
33
use crate::tools::utils::{is_absolute_or_tilde, is_http_url};
44
use base64::{Engine, engine::general_purpose::STANDARD};
5-
use image::{DynamicImage, GenericImageView, ImageEncoder, ImageFormat as ImageCrateFormat};
5+
use image::{
6+
DynamicImage, GenericImageView, ImageDecoder, ImageEncoder, ImageFormat as ImageCrateFormat,
7+
};
68
use std::collections::HashSet;
79
use std::path::{Path, PathBuf};
810
use std::sync::{Arc, Mutex};
@@ -39,11 +41,7 @@ pub struct EncodedImage {
3941
pub fn encode_image_for_prompt(bytes: Vec<u8>) -> Result<EncodedImage> {
4042
let detected = image::guess_format(&bytes).ok();
4143

42-
let decoded = image::load_from_memory(&bytes).map_err(|e| {
43-
SofosError::ToolExecution(format!(
44-
"Failed to decode image: {e}. Supported: {SUPPORTED_FORMATS_HUMAN_LIST}."
45-
))
46-
})?;
44+
let decoded = decode_with_orientation(&bytes)?;
4745
let (width, height) = decoded.dimensions();
4846

4947
let within_bound = width <= MAX_PROMPT_IMAGE_DIMENSION && height <= MAX_PROMPT_IMAGE_DIMENSION;
@@ -76,6 +74,26 @@ pub fn encode_image_for_prompt(bytes: Vec<u8>) -> Result<EncodedImage> {
7674
})
7775
}
7876

77+
/// Decode image bytes, applying any EXIF orientation. A resize +
78+
/// re-encode strips EXIF, so the orientation is baked into the pixels.
79+
fn decode_with_orientation(bytes: &[u8]) -> Result<DynamicImage> {
80+
fn decode_err<E: std::fmt::Display>(e: E) -> SofosError {
81+
SofosError::ToolExecution(format!(
82+
"Failed to decode image: {e}. Supported: {SUPPORTED_FORMATS_HUMAN_LIST}."
83+
))
84+
}
85+
86+
let mut decoder = image::ImageReader::new(std::io::Cursor::new(bytes))
87+
.with_guessed_format()
88+
.map_err(decode_err)?
89+
.into_decoder()
90+
.map_err(decode_err)?;
91+
let orientation = decoder.orientation().map_err(decode_err)?;
92+
let mut decoded = DynamicImage::from_decoder(decoder).map_err(decode_err)?;
93+
decoded.apply_orientation(orientation);
94+
Ok(decoded)
95+
}
96+
7997
fn is_passthrough_format(format: ImageCrateFormat) -> bool {
8098
matches!(
8199
format,
@@ -376,4 +394,63 @@ mod tests {
376394
"error should explain the failure; got: {msg}"
377395
);
378396
}
397+
398+
/// Build a JPEG carrying an EXIF orientation tag, so the decode
399+
/// path under test has a real orientation to read.
400+
fn jpeg_with_exif_orientation(
401+
width: u32,
402+
height: u32,
403+
rgba: [u8; 4],
404+
orientation: u16,
405+
) -> Vec<u8> {
406+
let base = encode_fixture(width, height, rgba, ImageCrateFormat::Jpeg);
407+
assert!(
408+
base.starts_with(&[0xFF, 0xD8]),
409+
"JPEG must open with the start-of-image marker"
410+
);
411+
412+
// EXIF holds the orientation in a little-endian TIFF block: a
413+
// byte-order marker, then a directory with one entry.
414+
let mut tiff = Vec::new();
415+
tiff.extend_from_slice(&[0x49, 0x49, 0x2A, 0x00]); // "II" marker + magic 42
416+
tiff.extend_from_slice(&8u32.to_le_bytes()); // byte offset of the directory
417+
tiff.extend_from_slice(&1u16.to_le_bytes()); // entry count
418+
tiff.extend_from_slice(&0x0112u16.to_le_bytes()); // orientation tag id
419+
tiff.extend_from_slice(&3u16.to_le_bytes()); // field type: 16-bit unsigned
420+
tiff.extend_from_slice(&1u32.to_le_bytes()); // value count
421+
tiff.extend_from_slice(&u32::from(orientation).to_le_bytes());
422+
tiff.extend_from_slice(&0u32.to_le_bytes()); // no following directory
423+
424+
let mut body = b"Exif\0\0".to_vec();
425+
body.extend_from_slice(&tiff);
426+
427+
let mut app1 = vec![0xFF, 0xE1]; // EXIF metadata marker
428+
// The segment length counts its own two length bytes.
429+
let segment_len = (2 + body.len()) as u16;
430+
app1.extend_from_slice(&segment_len.to_be_bytes());
431+
app1.extend_from_slice(&body);
432+
433+
let mut out = Vec::with_capacity(base.len() + app1.len());
434+
out.extend_from_slice(&base[..2]);
435+
out.extend_from_slice(&app1);
436+
out.extend_from_slice(&base[2..]);
437+
out
438+
}
439+
440+
#[test]
441+
fn applies_exif_orientation_when_decoding() {
442+
// A wide JPEG tagged "rotate 90°" must come back with its
443+
// dimensions swapped — the orientation is applied before the
444+
// resize + re-encode that would otherwise strip the EXIF tag.
445+
let bytes = jpeg_with_exif_orientation(8, 4, [180, 60, 60, 255], 6);
446+
let decoded = decode_with_orientation(&bytes).expect("decode oriented JPEG");
447+
assert_eq!(decoded.dimensions(), (4, 8));
448+
}
449+
450+
#[test]
451+
fn identity_exif_orientation_keeps_dimensions() {
452+
let bytes = jpeg_with_exif_orientation(8, 4, [180, 60, 60, 255], 1);
453+
let decoded = decode_with_orientation(&bytes).expect("decode JPEG");
454+
assert_eq!(decoded.dimensions(), (8, 4));
455+
}
379456
}

src/tools/permissions/scope.rs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ impl PermissionManager {
130130
)
131131
}
132132

133-
#[allow(dead_code)]
134133
pub fn check_write_permission(&self, path: &str) -> CommandPermission {
135134
self.check_scope_permission(
136135
path,
@@ -145,20 +144,10 @@ impl PermissionManager {
145144
self.is_scope_explicit_allow(path, Self::normalize_read, &self.read_allow_set)
146145
}
147146

148-
#[allow(dead_code)]
149-
pub fn is_read_explicit_allow_both_forms(&self, original: &str, canonical: &str) -> bool {
150-
self.is_read_explicit_allow(original) || self.is_read_explicit_allow(canonical)
151-
}
152-
153147
pub fn is_write_explicit_allow(&self, path: &str) -> bool {
154148
self.is_scope_explicit_allow(path, Self::normalize_write, &self.write_allow_set)
155149
}
156150

157-
#[allow(dead_code)]
158-
pub fn is_write_explicit_allow_both_forms(&self, original: &str, canonical: &str) -> bool {
159-
self.is_write_explicit_allow(original) || self.is_write_explicit_allow(canonical)
160-
}
161-
162151
/// Check if a path is covered by a Bash(/path/**) grant
163152
pub fn is_bash_path_allowed(&self, path: &str) -> bool {
164153
self.is_scope_explicit_allow(path, Self::normalize_command, &self.bash_path_allow_set)

0 commit comments

Comments
 (0)