fix(skills): a skill body stops at the 25k-token read cap
`read_file` exempted SKILL.md from every limit, and both injection paths (`/skill` expansion and the `skill` tool) pasted the whole body. A 220 KB skill put 55k tokens into the context on each call. `cap_skill_body` cuts an over-cap body at the last line boundary that fits (a char boundary when the body is one long line) and appends a note that points at offset and limit. It runs before `apply_substitutions`, so the arguments land after the cut. Both injection paths call it and log the cut. `read_file` returns a SKILL.md whole only while it fits the cap. Over the cap it reads like any other file: offset and limit apply, and a window that is still too large answers FileTooLarge. Without that the note would send the model to a read that ignores its range. `skill_file_skips_token_limit` pinned the exemption; it gives way to `skill_file_over_cap_returns_file_too_large` and `skill_file_over_cap_honors_offset_and_limit`, as upstream did. Ported from upstream (grok-build tools skills/skill.rs cap_skill_body and read_file skill_full, 1.0.33). Upstream's `max_output_bytes` read budget in the same hunk is a feature and stays out.
This commit is contained in:
6 files changed
+163
-28
No files matched your search
@@ -1020,6 +1020,7 @@ pub(super) async fn build_skill_information_for_refs(
|
||||
slash_skills: &[SkillInfo],
|
||||
session_id: &str,
|
||||
) -> Option<String> {
|
||||
use kigi_tools::implementations::skills::body_cap::cap_skill_body;
|
||||
use kigi_tools::implementations::skills::skill::{
|
||||
SkillRef, SubstitutionContext, apply_substitutions, build_skill_block,
|
||||
build_skill_information, load_skill_content,
|
||||
@@ -1034,6 +1035,13 @@ pub(super) async fn build_skill_information_for_refs(
|
||||
};
|
||||
match load_skill_content(info).await {
|
||||
Ok(mut content) => {
|
||||
if cap_skill_body(&mut content) {
|
||||
tracing::info!(
|
||||
skill = %sk.name,
|
||||
path = %info.path,
|
||||
"skill body truncated at read cap"
|
||||
);
|
||||
}
|
||||
let skill_dir = std::path::Path::new(&info.path)
|
||||
.parent()
|
||||
.and_then(|p| p.to_str());
|
||||
@@ -1465,6 +1473,31 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A 220 KB skill body stops at the read cap; the tail stays on disk.
|
||||
#[tokio::test]
|
||||
async fn build_skill_information_for_refs_caps_an_oversized_body() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("SKILL.md");
|
||||
let body = (1..=1100)
|
||||
.map(|n| format!("{n:05} {}", "x".repeat(194)))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
std::fs::write(&path, &body).unwrap();
|
||||
|
||||
let mut skill = make_skill("big", true);
|
||||
skill.path = path.to_string_lossy().to_string();
|
||||
let skills = vec![skill];
|
||||
|
||||
let parsed =
|
||||
parse_skill_references("/big", &skills, all_gated()).expect("known skill must parse");
|
||||
let info = build_skill_information_for_refs(&parsed, &skills, "sid-1")
|
||||
.await
|
||||
.expect("skill body must load");
|
||||
assert!(info.contains("00001 "), "head must survive");
|
||||
assert!(!info.contains("01100 "), "tail must be cut");
|
||||
assert!(info.contains("Skill body truncated at the 25000-token cap"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_loop_annotates_block_with_compact_display_text() {
|
||||
let outcome = resolve(
|
||||
|
||||
@@ -46,6 +46,12 @@ impl ReadFileVersion {
|
||||
}
|
||||
}
|
||||
pub(crate) const MAX_NUM_TOKENS: usize = 25_000;
|
||||
pub(crate) const MAX_NUM_BYTES: usize =
|
||||
MAX_NUM_TOKENS * kigi_token_estimation::BYTES_PER_TOKEN as usize;
|
||||
/// True when `text` is over the per-call token cap.
|
||||
pub(crate) fn exceeds_read_cap(text: &str) -> bool {
|
||||
crate::util::truncate::estimate_tokens(text) > MAX_NUM_TOKENS
|
||||
}
|
||||
pub const MAX_LINES_READ: usize = 1_000;
|
||||
pub use crate::implementations::read_file::{
|
||||
FileMetadata, PDF_MAX_PAGES_PER_READ, bytes_to_metadata, parse_page_range,
|
||||
@@ -434,22 +440,25 @@ pub(crate) async fn run_read_file(
|
||||
.map(|t| t.0.max_lines_read())
|
||||
.unwrap_or_else(|| TruncationConfig::default().max_lines_read())
|
||||
};
|
||||
let (effective_offset, effective_limit) = if is_skill_file {
|
||||
(None, None)
|
||||
} else {
|
||||
(
|
||||
input.offset,
|
||||
Some(input.limit.unwrap_or(usize::MAX).min(max_lines)),
|
||||
)
|
||||
// A skill file under the cap comes back whole; over it, it reads like any other file.
|
||||
let skill_full = is_skill_file
|
||||
.then(|| extract_file_content_lines(&file_content, None, None, total_lines))
|
||||
.filter(|full| !exceeds_read_cap(&full.content));
|
||||
let (extracted, stored_offset, stored_limit) = match skill_full {
|
||||
Some(full) => (full, None, None),
|
||||
None => (
|
||||
extract_file_content_lines(
|
||||
&file_content,
|
||||
input.offset,
|
||||
Some(input.limit.unwrap_or(usize::MAX).min(max_lines)),
|
||||
total_lines,
|
||||
),
|
||||
stored_read_offset(input.offset),
|
||||
input.limit,
|
||||
),
|
||||
};
|
||||
let extracted = extract_file_content_lines(
|
||||
&file_content,
|
||||
effective_offset,
|
||||
effective_limit,
|
||||
total_lines,
|
||||
);
|
||||
let token_count = crate::util::truncate::estimate_tokens(&extracted.content);
|
||||
if !is_skill_file && token_count > MAX_NUM_TOKENS {
|
||||
if exceeds_read_cap(&extracted.content) {
|
||||
let token_count = crate::util::truncate::estimate_tokens(&extracted.content);
|
||||
let (grep_name, execute_name);
|
||||
{
|
||||
let res = resources.lock().await;
|
||||
@@ -497,11 +506,6 @@ pub(crate) async fn run_read_file(
|
||||
};
|
||||
return Ok(ReadFileOutput::FileTooLarge(msg));
|
||||
}
|
||||
let (stored_offset, stored_limit) = if is_skill_file {
|
||||
(None, None)
|
||||
} else {
|
||||
(stored_read_offset(input.offset), input.limit)
|
||||
};
|
||||
if let Some(flag) = streamable_out {
|
||||
*flag = true;
|
||||
}
|
||||
@@ -1660,9 +1664,8 @@ pub fn verify(req: &HttpRequest) -> Result<Claims, Error> {
|
||||
other => panic!("Expected FileContent, got {:?}", other),
|
||||
}
|
||||
}
|
||||
#[tokio::test]
|
||||
async fn skill_file_skips_token_limit() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
/// 1100 lines of 200 bytes: about 55k tokens, over the 25k cap.
|
||||
fn write_oversized_skill(tmp: &TempDir) {
|
||||
let line = "x".repeat(200);
|
||||
let big_content = std::iter::repeat_n(line.as_str(), 1100)
|
||||
.collect::<Vec<_>>()
|
||||
@@ -1670,6 +1673,11 @@ pub fn verify(req: &HttpRequest) -> Result<Claims, Error> {
|
||||
let skill_dir = tmp.path().join("skills");
|
||||
std::fs::create_dir_all(&skill_dir).unwrap();
|
||||
std::fs::write(skill_dir.join("SKILL.md"), &big_content).unwrap();
|
||||
}
|
||||
#[tokio::test]
|
||||
async fn skill_file_over_cap_returns_file_too_large() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
write_oversized_skill(&tmp);
|
||||
let tool = ReadFileTool;
|
||||
let mut resources = test_resources(tmp.path());
|
||||
resources.insert(TemplateRenderer::new(
|
||||
@@ -1687,11 +1695,36 @@ pub fn verify(req: &HttpRequest) -> Result<Claims, Error> {
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(
|
||||
matches!(result, ReadFileOutput::FileContent(_)),
|
||||
"SKILL.md should not be truncated, got {:?}",
|
||||
matches!(result, ReadFileOutput::FileTooLarge(_)),
|
||||
"got {:?}",
|
||||
std::mem::discriminant(&result),
|
||||
);
|
||||
}
|
||||
#[tokio::test]
|
||||
async fn skill_file_over_cap_honors_offset_and_limit() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
write_oversized_skill(&tmp);
|
||||
let tool = ReadFileTool;
|
||||
let resources = test_resources(tmp.path());
|
||||
let input = ReadFileInput {
|
||||
path: "skills/SKILL.md".to_string(),
|
||||
offset: Some(3),
|
||||
limit: Some(2),
|
||||
pages: None,
|
||||
format: None,
|
||||
};
|
||||
let result = kigi_tool_runtime::Tool::run(&tool, test_ctx(resources.into_shared()), input)
|
||||
.await
|
||||
.unwrap();
|
||||
match result {
|
||||
ReadFileOutput::FileContent(fc) => {
|
||||
assert_eq!((Some(3), Some(2)), (fc.offset, fc.limit));
|
||||
let line = "x".repeat(200);
|
||||
assert_eq!(format!("3→{line}\n{line}"), fc.content);
|
||||
}
|
||||
other => panic!("Expected FileContent, got {:?}", other),
|
||||
}
|
||||
}
|
||||
#[test]
|
||||
fn parse_single_page() {
|
||||
assert_eq!(parse_page_range("3", 10).unwrap(), vec![2]);
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
|
||||
use std::path::Path;
|
||||
|
||||
use crate::implementations::skills::body_cap::cap_skill_body;
|
||||
use crate::implementations::skills::skill::{SkillOutput, extract_skill_body, format_skill_name};
|
||||
use crate::implementations::skills::types::SkillInfo;
|
||||
use crate::types::requirements::{Expr, ToolRequirement};
|
||||
@@ -279,7 +280,7 @@ impl kigi_tool_runtime::Tool for SkillTool {
|
||||
}
|
||||
};
|
||||
|
||||
let content = match load_skill_content(&skill).await {
|
||||
let mut content = match load_skill_content(&skill).await {
|
||||
Ok(c) => c,
|
||||
Err(e) => {
|
||||
return Ok(SkillOutput {
|
||||
@@ -292,6 +293,14 @@ impl kigi_tool_runtime::Tool for SkillTool {
|
||||
}
|
||||
};
|
||||
|
||||
if cap_skill_body(&mut content) {
|
||||
tracing::info!(
|
||||
skill = %skill.name,
|
||||
path = %skill.path,
|
||||
"skill body truncated at read cap"
|
||||
);
|
||||
}
|
||||
|
||||
let files = list_skill_files(&skill, 10).await;
|
||||
|
||||
let output = format_output(&skill, &content, &files);
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
//! Cap an injected skill body at the file-read token cap.
|
||||
|
||||
use crate::implementations::kigi::read_file::{MAX_NUM_BYTES, MAX_NUM_TOKENS, exceeds_read_cap};
|
||||
use crate::util::truncate::floor_char_boundary;
|
||||
|
||||
/// Cuts `body` on a line boundary under the read cap and appends a note; true when it cut.
|
||||
pub fn cap_skill_body(body: &mut String) -> bool {
|
||||
if !exceeds_read_cap(body) {
|
||||
return false;
|
||||
}
|
||||
let note = format!(
|
||||
"[Skill body truncated at the {MAX_NUM_TOKENS}-token cap. Read the rest of the \
|
||||
skill file with the file read tool using a line offset and limit.]"
|
||||
);
|
||||
let budget = MAX_NUM_BYTES.saturating_sub(note.len() + 2);
|
||||
let end = floor_char_boundary(body, budget);
|
||||
let cut = body[..end].rfind('\n').filter(|&i| i > 0).unwrap_or(end);
|
||||
body.truncate(cut);
|
||||
body.push_str("\n\n");
|
||||
body.push_str(¬e);
|
||||
true
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn leaves_body_under_cap_untouched() {
|
||||
let mut body = "# Small skill\n\nDo the thing.".to_owned();
|
||||
assert!(!cap_skill_body(&mut body));
|
||||
assert_eq!("# Small skill\n\nDo the thing.", body);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cuts_on_line_boundary_under_cap() {
|
||||
let mut body = (1..=1100)
|
||||
.map(|n| format!("{n:05} {}", "x".repeat(194)))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
|
||||
assert!(cap_skill_body(&mut body));
|
||||
assert!(!exceeds_read_cap(&body));
|
||||
assert!(!body.contains("01100 "));
|
||||
assert!(body.contains("offset"));
|
||||
let (head, _note) = body.rsplit_once("\n\n").expect("note separator");
|
||||
let last_line = head.rsplit('\n').next().expect("head has a line");
|
||||
assert_eq!(200, last_line.len());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_long_line_falls_back_to_char_cut() {
|
||||
let mut body = "é".repeat(60_000);
|
||||
|
||||
assert!(cap_skill_body(&mut body));
|
||||
assert!(!exceeds_read_cap(&body));
|
||||
assert!(body.starts_with("éé"));
|
||||
}
|
||||
}
|
||||
@@ -1,3 +1,4 @@
|
||||
pub mod body_cap;
|
||||
pub mod discovery;
|
||||
pub mod skill;
|
||||
pub mod types;
|
||||
@@ -8,8 +8,8 @@ const MAX_LINES_READ_DEFAULT: usize = 1_000;
|
||||
/// There is deliberately no per-line cap: clipping long lines silently
|
||||
/// corrupts single-line files (minified JSON, data dumps) with no way for
|
||||
/// the model to recover the clipped bytes. Non-skill reads are bounded by
|
||||
/// the whole-read `MAX_NUM_TOKENS` cap instead (skill files are exempt from
|
||||
/// all read limits by design). Other agent CLIs likewise apply no
|
||||
/// the whole-read `MAX_NUM_TOKENS` cap instead (a skill file under that cap
|
||||
/// comes back whole, offset and limit ignored). Other agent CLIs likewise apply no
|
||||
/// per-line cap. The wire field (`TruncationConfig.max_chars_per_line` in
|
||||
/// kigi-tools.proto) is deprecated and ignored.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
|
||||
Reference in new issue
Block a user