context accounting: one exhaustive counter, no wildcard arm
Three counters measured the same conversation and disagreed 5.6x: the /tokens endpoint said 315,487, the throttle ~200,000, and the compaction engine 56,361. Same TokenCounter, three different notions of what a block weighs, no labels anywhere. count_messages carried `_ => None`, so it saw Text and nothing else — 18% of a real conversation. Microcompact exists to blur old tool results and could not measure a single byte of the block kind it acts on. Worse, reclaimed = before - after against a blind counter is structurally always zero, so the "nothing to set down" branch always fired and the felt-state message written for a successful run was unreachable code. It could not distinguish "found nothing" from "worked perfectly and cannot say so". ContentBlock::countable_text is now the single authority, with a deliberately exhaustive match and no wildcard: a new block kind must fail to compile rather than quietly weigh nothing. The endpoint's inline copy is replaced by a call to it, since an inline copy is how the two drifted apart in the first place. Image still counts its base64 payload, which overstates real token cost. Preserved deliberately — re-weighting images is a model-specific estimate and a separate decision; doing both at once would make neither reviewable.
This commit is contained in:
parent
b9f404cbaf
commit
7530d6d1c8
3 changed files with 148 additions and 24 deletions
|
|
@ -962,25 +962,10 @@ pub async fn get_conversation_tokens(
|
|||
for msg in &session.messages {
|
||||
let mut msg_tokens = 0;
|
||||
for block in &msg.blocks {
|
||||
let block_text = match block {
|
||||
crate::core::session::ContentBlock::Text { text } => text.clone(),
|
||||
crate::core::session::ContentBlock::ToolUse { id, name, input } => {
|
||||
format!("{id} {name} {input}")
|
||||
}
|
||||
crate::core::session::ContentBlock::ToolResult {
|
||||
tool_use_id,
|
||||
tool_name,
|
||||
output,
|
||||
..
|
||||
} => {
|
||||
format!("{tool_use_id} {tool_name} {output}")
|
||||
}
|
||||
crate::core::session::ContentBlock::Reasoning { reasoning } => reasoning.clone(),
|
||||
crate::core::session::ContentBlock::Image { media_type, data } => {
|
||||
format!("{media_type} {data}")
|
||||
}
|
||||
};
|
||||
msg_tokens += counter.count(&block_text);
|
||||
// `countable_text` is the shared authority — this used to be an
|
||||
// inline copy of the same match, which is how the compaction
|
||||
// engine's copy was able to drift into counting Text only.
|
||||
msg_tokens += counter.count(&block.countable_text());
|
||||
}
|
||||
|
||||
match msg.role {
|
||||
|
|
|
|||
|
|
@ -20,15 +20,16 @@ const COMPACTABLE_TOOLS: &[&str] = &["read", "bash", "grep", "glob", "list_dir",
|
|||
const TIME_BASED_MC_CLEARED_MESSAGE: &str = "[Old tool result content cleared]";
|
||||
|
||||
/// Token-count a slice of messages using the bridge's TokenCounter.
|
||||
///
|
||||
/// Routes through `ContentBlock::countable_text`, the one authority for block
|
||||
/// weight. It previously had its own `_ => None` arm here and so counted Text
|
||||
/// only — meaning microcompact, which exists to clear old ToolResult content,
|
||||
/// could not see a single byte of what it was built to reclaim.
|
||||
pub fn count_messages(counter: &TokenCounter, messages: &[ConversationMessage]) -> usize {
|
||||
messages
|
||||
.iter()
|
||||
.flat_map(|m| &m.blocks)
|
||||
.filter_map(|b| match b {
|
||||
crate::core::session::ContentBlock::Text { text } => Some(text.as_str()),
|
||||
_ => None,
|
||||
})
|
||||
.map(|t| counter.count(t))
|
||||
.map(|b| counter.count(&b.countable_text()))
|
||||
.sum()
|
||||
}
|
||||
|
||||
|
|
@ -898,4 +899,102 @@ mod tests {
|
|||
"tool call and result must be kept together"
|
||||
);
|
||||
}
|
||||
|
||||
/// The bug that made microcompact a no-op for its entire existence.
|
||||
///
|
||||
/// `count_messages` carried `_ => None`, so it saw Text and nothing else.
|
||||
/// Microcompact's whole job is blurring old ToolResult output — the exact
|
||||
/// block kind it could not measure. So `before` and `after` were identical
|
||||
/// no matter what it cleared, `reclaimed` was always zero, and the report
|
||||
/// always took the "nothing to set down" branch. It could not distinguish
|
||||
/// "found nothing" from "worked perfectly and cannot say so".
|
||||
#[test]
|
||||
fn the_counter_sees_the_blocks_microcompact_exists_to_clear() {
|
||||
let counter = TokenCounter::new();
|
||||
|
||||
let tool_heavy = vec![ConversationMessage {
|
||||
role: MessageRole::Assistant,
|
||||
blocks: vec![
|
||||
ContentBlock::Text {
|
||||
text: "brief".to_string(),
|
||||
},
|
||||
ContentBlock::ToolUse {
|
||||
id: "call_1".to_string(),
|
||||
name: "read".to_string(),
|
||||
input: "a".repeat(400),
|
||||
},
|
||||
ContentBlock::ToolResult {
|
||||
tool_use_id: "call_1".to_string(),
|
||||
tool_name: "read".to_string(),
|
||||
output: "b".repeat(4000),
|
||||
is_error: false,
|
||||
},
|
||||
ContentBlock::Reasoning {
|
||||
reasoning: "c".repeat(400),
|
||||
},
|
||||
],
|
||||
usage: None,
|
||||
timestamp: None,
|
||||
}];
|
||||
|
||||
let counted = count_messages(&counter, &tool_heavy);
|
||||
|
||||
let text_only = counter.count("brief");
|
||||
assert!(
|
||||
counted > text_only * 10,
|
||||
"counter must weigh tool traffic, not just text: got {counted}, \
|
||||
text alone is {text_only}"
|
||||
);
|
||||
|
||||
// And the decisive property: clearing a tool result must be *visible*
|
||||
// to the counter, or the reclaim figure is structurally always zero.
|
||||
let mut cleared = tool_heavy.clone();
|
||||
cleared[0].blocks[2] = ContentBlock::ToolResult {
|
||||
tool_use_id: "call_1".to_string(),
|
||||
tool_name: "read".to_string(),
|
||||
output: TIME_BASED_MC_CLEARED_MESSAGE.to_string(),
|
||||
is_error: false,
|
||||
};
|
||||
let after = count_messages(&counter, &cleared);
|
||||
assert!(
|
||||
after < counted,
|
||||
"blurring a tool result must reduce the measured count \
|
||||
(before {counted}, after {after}) — otherwise microcompact \
|
||||
reports a no-op however much room it actually freed"
|
||||
);
|
||||
}
|
||||
|
||||
/// Every block kind must weigh something. A kind that counts as zero is
|
||||
/// invisible to compaction and to every pressure signal downstream.
|
||||
#[test]
|
||||
fn no_block_kind_weighs_nothing() {
|
||||
let counter = TokenCounter::new();
|
||||
let kinds = vec![
|
||||
ContentBlock::Text {
|
||||
text: "hello there".to_string(),
|
||||
},
|
||||
ContentBlock::ToolUse {
|
||||
id: "id".to_string(),
|
||||
name: "bash".to_string(),
|
||||
input: "some arguments here".to_string(),
|
||||
},
|
||||
ContentBlock::ToolResult {
|
||||
tool_use_id: "id".to_string(),
|
||||
tool_name: "bash".to_string(),
|
||||
output: "some output here".to_string(),
|
||||
is_error: false,
|
||||
},
|
||||
ContentBlock::Reasoning {
|
||||
reasoning: "thinking about it".to_string(),
|
||||
},
|
||||
ContentBlock::Image {
|
||||
media_type: "image/png".to_string(),
|
||||
data: "AAAABBBBCCCC".to_string(),
|
||||
},
|
||||
];
|
||||
for block in kinds {
|
||||
let weight = counter.count(&block.countable_text());
|
||||
assert!(weight > 0, "block kind weighed zero: {block:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -48,6 +48,46 @@ pub enum ContentBlock {
|
|||
},
|
||||
}
|
||||
|
||||
impl ContentBlock {
|
||||
/// Everything this block contributes to the context window, whatever its
|
||||
/// kind. This is the single authority for "how much room does this take" —
|
||||
/// every token counter in the system routes through it.
|
||||
///
|
||||
/// The match is deliberately exhaustive with **no wildcard arm**. A new
|
||||
/// block kind must fail to compile here rather than quietly weigh nothing.
|
||||
/// That is not stylistic: `count_messages` in the compaction engine carried
|
||||
/// `_ => None`, so it measured Text only — 18% of a real conversation — and
|
||||
/// microcompact, whose entire job is clearing old *tool results*, concluded
|
||||
/// there was nothing to reclaim and declined to run. A counter blind to the
|
||||
/// blocks its caller exists to act on reports success while doing nothing.
|
||||
///
|
||||
/// Known caveat, deliberately preserved rather than silently changed:
|
||||
/// `Image` counts its base64 payload as text, which vastly overstates a
|
||||
/// real image's token cost. Fixing that is a model-specific estimate and a
|
||||
/// separate decision — see the image-token follow-up. Unifying the counters
|
||||
/// and re-weighting images are two changes; doing both at once would make
|
||||
/// neither reviewable.
|
||||
pub fn countable_text(&self) -> std::borrow::Cow<'_, str> {
|
||||
use std::borrow::Cow;
|
||||
match self {
|
||||
ContentBlock::Text { text } => Cow::Borrowed(text),
|
||||
ContentBlock::ToolUse { id, name, input } => {
|
||||
Cow::Owned(format!("{id} {name} {input}"))
|
||||
}
|
||||
ContentBlock::ToolResult {
|
||||
tool_use_id,
|
||||
tool_name,
|
||||
output,
|
||||
..
|
||||
} => Cow::Owned(format!("{tool_use_id} {tool_name} {output}")),
|
||||
ContentBlock::Reasoning { reasoning } => Cow::Borrowed(reasoning),
|
||||
ContentBlock::Image { media_type, data } => {
|
||||
Cow::Owned(format!("{media_type} {data}"))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// An image attached to the current input, before submission.
|
||||
/// Used by the TUI input pipeline and carried through to the Backend trait.
|
||||
#[derive(Debug, Clone)]
|
||||
|
|
|
|||
Loading…
Reference in a new issue