Caveman L1 integration: port markdown compressor to Rust - #117
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a new ChangesCaveman Compression Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/caveman/src/compress.rs`:
- Around line 65-73: The retry path in compress handling is re-adding
frontmatter even though build_fix_prompt already returns the full fixed
compressed file, which can duplicate frontmatter and break markdown. Update the
logic around the compressed_body handling in compress.rs so the fix output is
used as-is, or adjust build_fix_prompt to return body-only content to match
build_compress_prompt; keep the behavior consistent in the compression and retry
flow.
- Around line 87-90: The `<source> <target>` handling in `compress.rs` can
destroy an existing target when validation fails because `std::fs::write`
replaces the file before the later cleanup removes it. Update the logic around
the target write path in the compression flow to use a temporary file or to back
up and restore any pre-existing target before swapping it in, and ensure the
failure cleanup only removes the newly created artifact rather than the original
target contents.
In `@crates/caveman/src/llm.rs`:
- Around line 50-80: The call_via_cli path can hang forever because
wait_with_output() has no deadline; add the same 120s timeout used by the HTTP
LLM path and make sure the spawned Command child is terminated if it expires.
Update call_via_cli in llm.rs to monitor the claude process, kill it on timeout,
and return a CavemanError::Llm that clearly reports the timeout for the claude
--print invocation.
In `@crates/caveman/src/validate.rs`:
- Around line 93-97: The heading validation in validate compares only the
lengths of extract_headings(orig) and extract_headings(comp), so renamed or
re-leveled headings can still pass. Update the validation logic in validate.rs
to compare the full heading tuples returned by extract_headings, not just the
counts, and keep the error reporting aligned with this stricter check. If you
adjust the mismatch message, also update the heading_count_mismatch_is_an_error
test to match the new text.
In `@src/cli/caveman.rs`:
- Around line 6-9: The `caveman` command docs and implementation are out of
sync: `--spec-file` is documented to default to a sibling backup, but the
`backup_mode` logic still falls back to `BackupMode::OutOfTree` when `--backup`
is omitted. Update the `backup_mode` match in `caveman` so it checks whether
`spec_file` is set and selects `BackupMode::Sibling` by default for that path,
while preserving the existing out-of-tree default for the non-`spec_file` flow.
Also keep the doc comment aligned with the actual behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e207e672-ec07-46d8-b138-7bdac4bdc6bb
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomlcrates/caveman/Cargo.tomlcrates/caveman/src/compress.rscrates/caveman/src/error.rscrates/caveman/src/frontmatter.rscrates/caveman/src/lib.rscrates/caveman/src/llm.rscrates/caveman/src/prompt.rscrates/caveman/src/sensitive.rscrates/caveman/src/validate.rssrc/cli/caveman.rssrc/cli/mod.rstests/caveman_cli.rs
Comment on lines
+65
to
+73
| let compressed_body = llm.call(&prompt.build_compress_prompt(&body))?; | ||
| if compressed_body.trim().is_empty() { | ||
| return Err(CavemanError::EmptyResponse); | ||
| } | ||
| if compressed_body.trim() == body.trim() { | ||
| return Err(CavemanError::IdenticalOutput); | ||
| } | ||
|
|
||
| let mut compressed = format!("{frontmatter_text}{compressed_body}"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep run --pattern 'fn build_fix_prompt($$$) { $$$ }' --lang rust crates/caveman/src/prompt.rs || \
rg -nP -C3 '\bbuild_fix_prompt\b' crates/caveman/src/prompt.rsRepository: getappz/agentflare
Length of output: 860
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- crates/caveman/src/prompt.rs ---'
sed -n '1,140p' crates/caveman/src/prompt.rs
echo
echo '--- crates/caveman/src/compress.rs (relevant section) ---'
sed -n '1,180p' crates/caveman/src/compress.rsRepository: getappz/agentflare
Length of output: 10101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' crates/caveman/src/validate.rsRepository: getappz/agentflare
Length of output: 6392
Avoid re-prepending frontmatter in the retry path. build_fix_prompt already returns the fixed compressed file, but this code wraps that output in frontmatter_text again. For files with frontmatter, that can duplicate it and produce malformed markdown. Return the fix output as-is, or make the retry prompt body-only to match the initial compression path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/caveman/src/compress.rs` around lines 65 - 73, The retry path in
compress handling is re-adding frontmatter even though build_fix_prompt already
returns the full fixed compressed file, which can duplicate frontmatter and
break markdown. Update the logic around the compressed_body handling in
compress.rs so the fix output is used as-is, or adjust build_fix_prompt to
return body-only content to match build_compress_prompt; keep the behavior
consistent in the compression and retry flow.
Comment on lines
+50
to
+80
| fn call_via_cli(prompt: &str) -> Result<String, CavemanError> { | ||
| let claude_bin = which::which("claude") | ||
| .map(|p| p.display().to_string()) | ||
| .unwrap_or_else(|_| "claude".to_string()); | ||
| let mut child = Command::new(&claude_bin) | ||
| .arg("--print") | ||
| .stdin(Stdio::piped()) | ||
| .stdout(Stdio::piped()) | ||
| .stderr(Stdio::piped()) | ||
| .spawn() | ||
| .map_err(|e| CavemanError::Llm(format!("spawn '{claude_bin}' failed: {e}")))?; | ||
| // Write stdin on a separate thread, concurrently with wait_with_output() | ||
| // draining stdout/stderr below — writing the whole prompt first and only | ||
| // then waiting would deadlock if the child fills its stdout/stderr pipe | ||
| // buffer before finishing reading stdin (both sides then block forever). | ||
| let mut stdin = child.stdin.take().expect("stdin was piped"); | ||
| let prompt_owned = prompt.to_string(); | ||
| let writer = std::thread::spawn(move || stdin.write_all(prompt_owned.as_bytes())); | ||
| let output = child | ||
| .wait_with_output() | ||
| .map_err(|e| CavemanError::Llm(format!("'{claude_bin}' failed: {e}")))?; | ||
| let write_result = writer | ||
| .join() | ||
| .map_err(|_| CavemanError::Llm(format!("stdin writer thread for '{claude_bin}' panicked")))?; | ||
| write_result.map_err(|e| CavemanError::Llm(format!("write to '{claude_bin}' stdin failed: {e}")))?; | ||
| if !output.status.success() { | ||
| let stderr = String::from_utf8_lossy(&output.stderr); | ||
| return Err(CavemanError::Llm(format!("Claude call failed:\n{stderr}"))); | ||
| } | ||
| Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
sed -n '1,180p' crates/caveman/src/llm.rsRepository: getappz/agentflare
Length of output: 4730
🏁 Script executed:
rg -n "call_via_cli|wait_with_output|timeout|cancel|ctrl_c|kill\(" crates -g '!target'Repository: getappz/agentflare
Length of output: 10027
Add a timeout for the claude subprocess. wait_with_output() can block forever if claude --print hangs, since this path has no watchdog or cancellation. Mirror the HTTP path’s 120s deadline and kill the child on expiry.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/caveman/src/llm.rs` around lines 50 - 80, The call_via_cli path can
hang forever because wait_with_output() has no deadline; add the same 120s
timeout used by the HTTP LLM path and make sure the spawned Command child is
terminated if it expires. Update call_via_cli in llm.rs to monitor the claude
process, kill it on timeout, and return a CavemanError::Llm that clearly reports
the timeout for the claude --print invocation.
Comment on lines
+6
to
+9
| /// Compress a markdown file. With no --spec-file, uses caveman's own | ||
| /// generic compression prompt and backs up out-of-tree. With | ||
| /// --spec-file, uses the given spec text as the compression prompt | ||
| /// (used by short-skill) and defaults to a sibling backup. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Doc comment promises a sibling-backup default for --spec-file that the code doesn't implement.
The doc comment says: --spec-file, uses the given spec text ... and defaults to a sibling backup. But the backup_mode match (lines 42-49) never inspects spec_file — when --backup is omitted it always resolves to BackupMode::OutOfTree, regardless of whether spec_file is set. This is a functional mismatch between documented behavior and the implementation; a caller invoking --spec-file without an explicit --backup gets OutOfTree instead of the promised Sibling.
🐛 Proposed fix to make backup default depend on spec_file
let backup_mode = match backup.as_deref() {
Some("sibling") => caveman::BackupMode::Sibling,
- Some("out-of-tree") | None => caveman::BackupMode::OutOfTree,
+ Some("out-of-tree") => caveman::BackupMode::OutOfTree,
+ None => if spec_file.is_some() {
+ caveman::BackupMode::Sibling
+ } else {
+ caveman::BackupMode::OutOfTree
+ },
Some(other) => {
eprintln!("--backup must be 'sibling' or 'out-of-tree', got '{other}'");
std::process::exit(1);
}
};Also applies to: 42-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/caveman.rs` around lines 6 - 9, The `caveman` command docs and
implementation are out of sync: `--spec-file` is documented to default to a
sibling backup, but the `backup_mode` logic still falls back to
`BackupMode::OutOfTree` when `--backup` is omitted. Update the `backup_mode`
match in `caveman` so it checks whether `spec_file` is set and selects
`BackupMode::Sibling` by default for that path, while preserving the existing
out-of-tree default for the non-`spec_file` flow. Also keep the doc comment
aligned with the actual behavior.
…ath collisions - validate(): compare heading (level, text) tuples, not just h1.len() vs h2.len() — a same-count-but-reworded heading previously passed silently, even though the compress/fix prompts both promise exact heading preservation. - backup_path_for(OutOfTree): hash the full canonicalized parent path instead of just its last component — two files with the same name under differently-located but identically-named parent dirs (e.g. project-a/docs/README.md and project-b/docs/README.md) previously collided on the same backup path. Both covered by new tests; full workspace test suite passes.
3 tasks
Merged
This was referenced Jul 11, 2026
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
caveman-compressmarkdown/prose LLM-compressor into a new Rust workspace cratecrates/caveman(agentflare-caveman), mirroring howponytailwas similarly ported ("L1 integration"): frontmatter/sensitive-path/structural-validation/LLM-invocation/orchestration all as small, independently-tested modules.agentflare caveman compressCLI subcommand —agentflare caveman compress <file>for generic compression, or--spec-file <path> --backup sibling <source> <target>for a caller-supplied compression spec (used by short-skill).claudeCLI, and an integer-underflow panic in the CLI's percentage display when an LLM response is larger than its input.~/.claude/skills/short-skill/resolve.py, thecaveman-compressplugin's owncli.py) to shell out to the new Rust command instead — the old Pythoncompress.py/validate.pyare deleted. These two edits are outside this repository (user-owned Claude Code skill/plugin files) and aren't part of this diff, but are the reason this crate exists.Test plan
cargo build --workspacecargo test --workspace(fully green — 30 new tests incrates/cavemanplus a real end-to-end CLI integration test that drives the actual compiled binary against a stubbedclaudeexecutable on PATH)Summary by CodeRabbit
caveman compresscommand to compress Markdown with in-place editing or writing to a separate target.claudeexecutable.