Skip to content

feat: Integrate DSRs for automatic SYSTEM prompt optimization (#45)#61

Merged
madclaws merged 11 commits intotilesprivacy:mainfrom
bxff:feature/dsrs-integration
Jan 21, 2026
Merged

feat: Integrate DSRs for automatic SYSTEM prompt optimization (#45)#61
madclaws merged 11 commits intotilesprivacy:mainfrom
bxff:feature/dsrs-integration

Conversation

@bxff
Copy link
Copy Markdown
Contributor

@bxff bxff commented Jan 14, 2026

Description:

This PR brings DSRs (dspy-rs) into the tiles CLI to automatically optimize SYSTEM prompts in Modelfiles.

What it does

The new optimize subcommand refines your SYSTEM prompts using Chain of Thought reasoning and the COPRO optimizer. Give it a Modelfile with a starting SYSTEM prompt, and it'll rewrite it to be more effective.

Basic usage:

tiles optimize path/to/your.Modelfile

By default, the command:

  • Generates 5 synthetic examples automatically if you don't provide training data
  • Uses openai:gpt-4o-mini as the optimization model
  • Updates your Modelfile in-place with the improved SYSTEM prompt

Options:

  • --data <path>: Provide your own training data as a JSON file
  • --model <model>: Specify a different model (format: provider:model-name)

Important: Your Modelfile must contain a starting SYSTEM prompt so the optimizer understands what you're trying to accomplish.

Training data format

If you want to use your own examples instead of synthetic ones, create a JSON file like this:

[
  {
    "input": "User query here",
    "output": "Expected AI response here"
  }
]

API key you'll need

Make sure you have the right environment variable set for whichever model you're using:

  • OpenAI models: OPENAI_API_KEY
  • Anthropic (Claude): ANTHROPIC_API_KEY
  • Gemini: GEMINI_API_KEY
  • Groq: GROQ_API_KEY
  • OpenRouter: OPENROUTER_API_KEY

Quick example

Let's say you start with this Modelfile:

FROM llama3
SYSTEM "You are a helpful assistant."

To optimize it with Claude:

export ANTHROPIC_API_KEY=sk-ant-...
tiles optimize my.Modelfile --model anthropic:claude-3-5-sonnet-20240620

The command will rewrite your SYSTEM prompt with a more effective version based on the optimizer's evaluation.

Technical details

  • Uses dspy-rs v0.7.3 under the hood
  • Runs COPRO (Coordinate Prompt Optimization) with breadth=5, depth=2
  • Evaluates prompts with a multi-factor heuristic (length, structure, persona) that doesn't incur extra API costs
  • Fixed the tilekit Modelfile writer to properly persist SYSTEM prompt changes
  • Manually implements the Optimizable trait for PromptOptimizerModule

The optimization process typically takes a few minutes depending on the model and dataset size.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

Adds a new prompt optimization feature: a tilekit::optimize module with an async optimize(...) entry point that reads a Modelfile, configures an LM adapter, loads or synthesizes training examples, runs a COPRO-based optimization loop to produce an improved SYSTEM prompt, and writes the updated prompt back into the Modelfile. modelfile.rs’s add_system now replaces or appends the SYSTEM line without erroring on duplicates. Exposes an Optimize CLI subcommand (with modelfile_path, optional data, and model options), re-exports optimize in tiles commands, adds an optimization JSON fixture, new dependencies, and tests for metric/evaluator logic.

Sequence Diagram

sequenceDiagram
    actor User
    participant CLI as CLI (tiles/main.rs)
    participant Commands as Commands (tiles/commands/mod.rs)
    participant Optimize as Optimize Module (tilekit/optimize.rs)
    participant Modelfile as Modelfile (tilekit/src/modelfile.rs)
    participant Data as Data Loader (optimize.rs)
    participant LM as LM Adapter (dspy-rs)
    participant COPRO as COPRO Optimizer (dspy-rs)

    User->>CLI: run optimize command
    CLI->>Commands: dispatch Optimize
    Commands->>Optimize: optimize(modelfile_path, data, model)
    Optimize->>Modelfile: read Modelfile
    Modelfile-->>Optimize: content (including SYSTEM)
    Optimize->>Data: load or generate training examples
    Data-->>Optimize: examples
    Optimize->>LM: configure model/adapter
    LM-->>Optimize: configured predictor
    Optimize->>COPRO: build PromptOptimizerModule with predictor
    COPRO->>COPRO: run optimization with examples
    COPRO-->>Optimize: optimized SYSTEM prompt
    Optimize->>Modelfile: replace/append SYSTEM line
    Modelfile-->>Optimize: updated Modelfile
    Optimize-->>CLI: return result
    CLI-->>User: print outcome
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

  • tilesprivate/tiles#45: Implements Modelfile SYSTEM prompt optimization using dspy-rs and COPRO.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 39.13% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: Integrate DSRs for automatic SYSTEM prompt optimization (#45)' accurately and concisely describes the main change—integrating dspy-rs (DSRs) for optimizing SYSTEM prompts in Modelfiles.
Description check ✅ Passed The description provides comprehensive and relevant information about the new optimize subcommand, including usage, options, training data format, API requirements, and technical details about the implementation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@feynon feynon requested a review from madclaws January 14, 2026 12:57
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@tilekit/src/modelfile.rs`:
- Around line 182-196: The update_system method currently writes SYSTEM "value"
without escaping embedded double quotes, which will break parsing; modify
update_system (and the logic that writes into self.data) to detect if value
contains double quotes and either escape embedded quotes (e.g., replace " with
\") before formatting or switch to a safe delimiter such as a triple-quote block
(e.g., SYSTEM """...""") for values with special characters, and ensure both the
find/replace branch and the push branch use the same escaped/quoted output.
- Around line 169-180: The add_system method mutates self.data before validating
whether a SYSTEM already exists, leaving self.data inconsistent on Err; modify
add_system so it first checks self.system.is_some() and returns the Err without
changing state, and only after confirming no existing SYSTEM set self.system =
Some(value.to_owned()) and then push the formatted string into self.data; refer
to the add_system function, self.system field and self.data vector when making
this change.

In `@tiles/src/commands/optimize.rs`:
- Around line 180-181: The code silently drops errors from
SystemPromptSignature::update_instruction by calling
sig.update_instruction(system_prompt).unwrap_or_default(); replace this with
proper error handling: match or use the ? operator to propagate the update error
(e.g., sig.update_instruction(system_prompt)? or map the error to the function's
error type with context), or if propagation isn't appropriate, log the error
with context (including system_prompt) and return an Err; ensure you reference
SystemPromptSignature::new and sig.update_instruction(system_prompt) when making
the change.
🧹 Nitpick comments (2)
tiles/src/commands/optimize.rs (2)

90-90: Consider returning Result for error propagation to CLI.

The optimize function returns () and uses eprintln! + early return for all error cases. This prevents the calling CLI from detecting failures programmatically (e.g., for exit codes or error aggregation).

♻️ Suggested signature change
-pub async fn optimize(modelfile_path: String, data_path: Option<String>, model: String) {
+pub async fn optimize(modelfile_path: String, data_path: Option<String>, model: String) -> anyhow::Result<()> {

Then replace eprintln!(...); return; patterns with anyhow::bail!(...) or return Err(...).


134-167: Silent fallback to empty vec on data errors may mask issues.

When data file reading/parsing fails (lines 140-142, 153-156), the code logs an error but returns an empty vec![]. Combined with the check on line 169, this exits gracefully—but the user sees two messages ("Error parsing..." then "No training examples...") which could be confusing.

Consider propagating errors or making the fallback behavior more explicit.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9181c9a and 9794bf5.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • tilekit/src/modelfile.rs
  • tiles/Cargo.toml
  • tiles/fixtures/optimize_data.json
  • tiles/src/commands/mod.rs
  • tiles/src/commands/optimize.rs
  • tiles/src/main.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{rs,toml}

⚙️ CodeRabbit configuration file

Review the Rust code for conformity with best practices in Rust, Systems programming. Highlight any deviations.

Files:

  • tiles/Cargo.toml
  • tiles/src/main.rs
  • tiles/src/commands/mod.rs
  • tilekit/src/modelfile.rs
  • tiles/src/commands/optimize.rs
🧬 Code graph analysis (3)
tiles/src/main.rs (1)
tiles/src/commands/optimize.rs (1)
  • optimize (90-208)
tiles/src/commands/mod.rs (1)
tiles/src/commands/optimize.rs (1)
  • optimize (90-208)
tiles/src/commands/optimize.rs (1)
tilekit/src/modelfile.rs (4)
  • new (88-90)
  • new (107-119)
  • from_str (64-71)
  • from_str (251-253)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (8)
tiles/fixtures/optimize_data.json (1)

1-14: LGTM!

The fixture provides valid JSON training data with diverse example types (summarization, code generation, factual QA). The structure correctly matches the TrainingExample schema expected by optimize.rs.

tiles/Cargo.toml (2)

18-20: LGTM on new dependencies.

The added dependencies align with the optimization workflow:

  • dspy_rs for COPRO optimizer and prediction framework
  • bon for builder pattern derivation
  • indexmap for ordered parameter maps in Optimizable trait

4-4: Edition 2024 is official and stable. Rust 2024 Edition was released as stable in Rust 1.85 (February 2025), so your usage is supported. The workspace consistently uses it across tiles and tilekit Cargo.toml files. No action required.

Likely an incorrect or invalid review comment.

tiles/src/commands/optimize.rs (2)

57-88: Heuristic metric is reasonable for cost-optimization.

The multi-factor scoring (non-empty, length bounds, structure, persona keywords) provides a cost-free evaluation alternative to LLM-as-judge. The maximum score correctly sums to 1.0.

One consideration: the metric evaluates ai_response content rather than prompt quality directly. This assumes better prompts produce responses with these characteristics, which may not always correlate.


210-239: Synthetic data generation looks correct.

The function properly:

  • Uses the SyntheticDataSignature predictor
  • Strips markdown code fences from LLM output
  • Propagates JSON parsing errors via ?
  • Maps to the expected Example format
tiles/src/commands/mod.rs (1)

8-9: LGTM!

The module wiring correctly exposes optimize at the commands level, consistent with other command patterns in this module.

tiles/src/main.rs (2)

33-46: LGTM! Clean CLI subcommand definition.

The new Optimize variant follows existing patterns in the codebase. The argument configuration is appropriate:

  • Required modelfile_path for the target file
  • Optional data with convenient short flag
  • Sensible default for model

115-121: LGTM! Dispatch follows established pattern.

The match arm correctly destructures and passes parameters to commands::optimize. The error handling approach (internal to the optimize function) is consistent with other commands in this file.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment thread tilekit/src/modelfile.rs Outdated
Comment thread tilekit/src/modelfile.rs Outdated
Comment thread tiles/src/commands/optimize.rs Outdated
Comment on lines +180 to +181
let mut sig = SystemPromptSignature::new();
sig.update_instruction(system_prompt).unwrap_or_default();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Silently ignoring update_instruction errors.

unwrap_or_default() discards any error from update_instruction. If the signature update fails, the optimization proceeds with the default instruction, which may produce unexpected results.

💡 Suggested handling
 let mut sig = SystemPromptSignature::new();
-sig.update_instruction(system_prompt).unwrap_or_default();
+if let Err(e) = sig.update_instruction(system_prompt) {
+    eprintln!("Warning: Failed to set initial instruction: {}", e);
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let mut sig = SystemPromptSignature::new();
sig.update_instruction(system_prompt).unwrap_or_default();
let mut sig = SystemPromptSignature::new();
if let Err(e) = sig.update_instruction(system_prompt) {
eprintln!("Warning: Failed to set initial instruction: {}", e);
}
🤖 Prompt for AI Agents
In `@tiles/src/commands/optimize.rs` around lines 180 - 181, The code silently
drops errors from SystemPromptSignature::update_instruction by calling
sig.update_instruction(system_prompt).unwrap_or_default(); replace this with
proper error handling: match or use the ? operator to propagate the update error
(e.g., sig.update_instruction(system_prompt)? or map the error to the function's
error type with context), or if propagation isn't appropriate, log the error
with context (including system_prompt) and return an Err; ensure you reference
SystemPromptSignature::new and sig.update_instruction(system_prompt) when making
the change.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@tiles/src/commands/optimize.rs`:
- Around line 134-167: The code currently swallows JSON parse/read errors when
building `examples`, which masks malformed user data; change the `if let
Some(path) = data_path` branch so that on `fs::read_to_string` or
`serde_json::from_str` failures you return or propagate an error (or exit with a
nonzero status) instead of printing and returning an empty `vec![]`. Locate the
`examples` construction (the `if let Some(path) = data_path` match and the
`serde_json::from_str` error arm) and replace the `eprintln!` + `vec![]` with an
immediate error return (or `std::process::exit(1)` / `anyhow::bail!`) so callers
see a clear failure when a provided `--data <path>` file is malformed rather
than falling through to the empty-example path that calls
`generate_synthetic_examples`.
♻️ Duplicate comments (1)
tilekit/src/modelfile.rs (1)

183-201: Consider handling newlines for multiline SYSTEM prompts.

The quote escaping is good. However, if value contains newlines, the single-quoted format SYSTEM "..." may not parse correctly per the Modelfile grammar (which uses triple-quotes for multiline strings).

💡 Optional enhancement for multiline support
 pub fn update_system(&mut self, value: &str) {
     self.system = Some(value.to_owned());
-    // Escape double quotes to prevent breaking the Modelfile format
-    let escaped_value = value.replace('"', "\\\"");
-    let formatted = format!("SYSTEM \"{}\"", escaped_value);
+    let formatted = if value.contains('\n') {
+        format!("SYSTEM \"\"\"{}\"\"\"", value)
+    } else {
+        let escaped_value = value.replace('"', "\\\"");
+        format!("SYSTEM \"{}\"", escaped_value)
+    };
🧹 Nitpick comments (1)
tiles/src/commands/optimize.rs (1)

57-88: Metric ignores expected output from training examples.

The evaluator scores responses purely on heuristic characteristics (length, structure, persona keywords) without comparing to the _example's expected output. This means the optimizer favors stylistic patterns over semantic correctness.

This appears intentional per the PR description (avoiding extra API costs), but consider documenting this trade-off explicitly in a comment.

 impl Evaluator for PromptOptimizerModule {
+    // Note: This metric evaluates response quality heuristically without comparing
+    // to expected outputs, avoiding additional API calls for semantic evaluation.
     async fn metric(&self, _example: &Example, prediction: &Prediction) -> f32 {
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9794bf5 and 1f34dac.

📒 Files selected for processing (2)
  • tilekit/src/modelfile.rs
  • tiles/src/commands/optimize.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{rs,toml}

⚙️ CodeRabbit configuration file

Review the Rust code for conformity with best practices in Rust, Systems programming. Highlight any deviations.

Files:

  • tilekit/src/modelfile.rs
  • tiles/src/commands/optimize.rs
🧬 Code graph analysis (1)
tiles/src/commands/optimize.rs (1)
tilekit/src/modelfile.rs (2)
  • new (88-90)
  • new (107-119)
🔇 Additional comments (4)
tilekit/src/modelfile.rs (1)

169-181: LGTM! Quote escaping properly implemented.

The add_system method now correctly validates before mutating state, and escapes embedded double quotes. This addresses previous review feedback.

tiles/src/commands/optimize.rs (3)

1-41: LGTM! Well-structured DSPy integration.

The signature definitions and module structure follow dspy-rs patterns appropriately. The Builder derive for PromptOptimizerModule provides clean construction.


180-187: Good error handling for instruction update.

The previous review feedback about silently ignoring update_instruction errors has been properly addressed with explicit error logging and early return.


216-245: LGTM! Clean synthetic data generation.

The markdown cleanup logic is pragmatic for handling LLM responses that may wrap JSON in code blocks. Error propagation via ? is appropriate.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment thread tilekit/src/optimize.rs
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@tiles/src/commands/optimize.rs`:
- Around line 57-88: The metric currently ignores the example and only scores
prediction heuristics; update PromptOptimizerModule::metric to extract the
ground-truth ai_response from the provided Example (e.g.,
example.get("ai_response", None) or equivalent), compute a similarity between
the ground-truth and the prediction (exact match -> boost, otherwise
fuzzy/substring/ROUGE-like score normalized 0.0–1.0), and combine that
similarity with the existing heuristic checks (length, structure, persona) into
the final floating-point score returned by metric so the optimizer receives
correctness signals as well as formatting signals.
🧹 Nitpick comments (2)
tiles/src/commands/optimize.rs (2)

164-168: Inconsistent error handling for synthetic example generation.

When loading from a data file, parse errors now correctly fail fast (lines 147-149). However, when synthetic example generation fails, the error is logged but execution continues with an empty vector before exiting at line 171-174.

For consistency, consider failing immediately here as well:

💡 Suggested fix
     } else {
         println!("No training data provided. Generating synthetic examples...");
         match generate_synthetic_examples(&system_prompt).await {
             Ok(exs) => exs,
             Err(e) => {
                 eprintln!("Failed to generate synthetic examples: {}", e);
-                vec![]
+                return;
             }
         }
     };
-
-    if examples.is_empty() {
-        println!("No training examples available. Cannot optimize effectively.");
-        return;
-    }

195-195: Consider making optimizer parameters configurable.

The COPRO optimizer uses hardcoded breadth(5) and depth(2). For flexibility, these could be exposed as CLI options, especially since optimization time depends on these values.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f34dac and 5c3bd51.

📒 Files selected for processing (1)
  • tiles/src/commands/optimize.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{rs,toml}

⚙️ CodeRabbit configuration file

Review the Rust code for conformity with best practices in Rust, Systems programming. Highlight any deviations.

Files:

  • tiles/src/commands/optimize.rs
🧬 Code graph analysis (1)
tiles/src/commands/optimize.rs (1)
tilekit/src/modelfile.rs (2)
  • new (88-90)
  • new (107-119)
🔇 Additional comments (5)
tiles/src/commands/optimize.rs (5)

1-15: LGTM!

Imports are well-organized and the TrainingExample struct is appropriately minimal for its deserialization purpose.


17-35: LGTM!

The signature structs are well-defined with clear docstrings that guide the LM behavior appropriately.


37-56: LGTM!

The PromptOptimizerModule correctly implements the Module and Optimizable traits, with proper delegation to the inner Predict instance.


182-214: LGTM on the optimization flow.

The error handling for update_instruction is now properly implemented with context logging (addressing the previous review comment). The COPRO compilation, instruction extraction, and Modelfile update are all correctly sequenced.


228-247: Markdown cleanup and parsing look good.

The trimming of markdown code fences (```json and ```) is a practical approach to handle common LLM response formatting. The conversion logic properly maps TrainingExample to Example using the example! macro.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment thread tilekit/src/optimize.rs
Comment thread tilekit/src/optimize.rs Outdated
Comment on lines +224 to +236
let prediction = predictor.forward(input).await?;
let field = prediction.get("synthetic_data", None);
let synthetic_json = field.as_str().unwrap_or("");

// Clean up potential markdown formatting
let clean_json = synthetic_json
.trim()
.trim_start_matches("```json")
.trim_start_matches("```")
.trim_end_matches("```")
.trim();

let data: Vec<TrainingExample> = serde_json::from_str(clean_json)?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Silent fallback to empty string if synthetic_data field is missing.

At line 226, if the prediction doesn't contain the expected synthetic_data field or it's not a string, unwrap_or("") silently produces an empty string. The subsequent serde_json::from_str(clean_json) will then fail with a cryptic parse error.

Consider providing a more informative error:

💡 Suggested fix
     let prediction = predictor.forward(input).await?;
     let field = prediction.get("synthetic_data", None);
-    let synthetic_json = field.as_str().unwrap_or("");
+    let synthetic_json = field.as_str().ok_or_else(|| {
+        anyhow::anyhow!("LLM did not return expected 'synthetic_data' field")
+    })?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let prediction = predictor.forward(input).await?;
let field = prediction.get("synthetic_data", None);
let synthetic_json = field.as_str().unwrap_or("");
// Clean up potential markdown formatting
let clean_json = synthetic_json
.trim()
.trim_start_matches("```json")
.trim_start_matches("```")
.trim_end_matches("```")
.trim();
let data: Vec<TrainingExample> = serde_json::from_str(clean_json)?;
let prediction = predictor.forward(input).await?;
let field = prediction.get("synthetic_data", None);
let synthetic_json = field.as_str().ok_or_else(|| {
anyhow::anyhow!("LLM did not return expected 'synthetic_data' field")
})?;
// Clean up potential markdown formatting
let clean_json = synthetic_json
.trim()
.trim_start_matches("

Comment thread tilekit/src/optimize.rs
@@ -0,0 +1,272 @@
use bon::Builder;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move the optimize module as part of tilekit crate instead of tiles?

Comment thread tilekit/src/optimize.rs
@@ -0,0 +1,272 @@
use bon::Builder;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add tests for this module?

Comment thread tilekit/src/modelfile.rs Outdated
}
}

pub fn update_system(&mut self, value: &str) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of update_system function, we can just use add_system, where we can just remove the check and just overwrite the existing system prompt.

Comment thread tilekit/src/optimize.rs Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@tilekit/src/optimize.rs`:
- Around line 243-290: The tests module is placed before the function
generate_synthetic_examples which triggers the items_after_test_module Clippy
lint; move the generate_synthetic_examples function (or any other non-test
items) above the #[cfg(test)] mod tests block (referencing
generate_synthetic_examples and the tests module) so non-test items are declared
before the test module, or alternatively add a
#[allow(clippy::items_after_test_module)] on the tests module to silence the
lint.
♻️ Duplicate comments (1)
tilekit/src/optimize.rs (1)

298-301: Return an explicit error when synthetic_data is missing.
unwrap_or("") hides the root cause and surfaces a confusing JSON parse error.

💡 Suggested fix
     let prediction = predictor.forward(input).await?;
     let field = prediction.get("synthetic_data", None);
-    let synthetic_json = field.as_str().unwrap_or("");
+    let synthetic_json = field
+        .as_str()
+        .ok_or_else(|| anyhow::anyhow!("LLM did not return expected 'synthetic_data' field"))?;

Comment thread tilekit/src/optimize.rs Outdated
Comment thread tilekit/src/modelfile.rs Outdated
} else {
self.system = Some(value.to_owned());
Ok(())
format!("SYSTEM \"{}\"", value)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
format!("SYSTEM \"{}\"", value)
format!("SYSTEM {}", value)

@madclaws
Copy link
Copy Markdown
Member

Can we fix the CI @bxff ?

@madclaws
Copy link
Copy Markdown
Member

Can we add more tests to the optimize.rs module, basically tests for the optimize function.

Comment thread tilekit/src/optimize.rs Outdated
Comment on lines +237 to +241
match fs::write(&modelfile_path, modelfile.to_string()) {
Ok(_) => println!("Successfully updated {}", modelfile_path),
Err(e) => eprintln!("Error writing Modelfile: {}", e),
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also im of the opinion that we should return the Modelfile and lets the caller do what they want, like write or update the modelfile. Since this function right now doesn't give the flexibility to do more.

@madclaws madclaws linked an issue Jan 20, 2026 that may be closed by this pull request
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@tiles/src/main.rs`:
- Around line 115-123: The CLI currently logs file write errors in the
Commands::Optimize branch but swallows them, returning Ok(()) so the process
exits zero; change the match handling around std::fs::write in that branch
(inside the Commands::Optimize arm calling commands::optimize) to propagate
failures instead of only printing: on Err(e) return Err(e.into()) (or map the
io::Error into your main Result type) so the async main (or the caller of
commands::optimize) receives the error and the process exits non‑zero; adjust
the surrounding function signature to return a Result if needed.
♻️ Duplicate comments (2)
tilekit/src/modelfile.rs (1)

169-184: Preserve valid SYSTEM formatting for multiline/quoted prompts.

SYSTEM {value} will break when value contains newlines or quotes (likely for optimized prompts), producing invalid Modelfiles on re-parse. Consider switching to a quoted or triple‑quoted format when needed.

🛠️ Proposed fix
-        let formatted = format!("SYSTEM {}", value);
+        let formatted = if value.contains('\n') || value.contains('"') {
+            format!("SYSTEM \"\"\"{}\"\"\"", value)
+        } else {
+            format!("SYSTEM {}", value)
+        };
tilekit/src/optimize.rs (1)

217-229: Return a targeted error when synthetic_data is missing.

unwrap_or("") turns a missing field into a JSON parse error, which is hard to diagnose. Emit a clearer error instead.

🛠️ Proposed fix
-    let synthetic_json = field.as_str().unwrap_or("");
+    let synthetic_json = field.as_str().ok_or_else(|| {
+        anyhow::anyhow!("LLM did not return expected 'synthetic_data' field")
+    })?;

Comment thread tiles/src/main.rs Outdated
bxff added 2 commits January 21, 2026 04:06
…them

The Commands::Optimize branch was logging errors but returning Ok(()),
causing the process to exit with code 0 even on failure. Now uses ? to
propagate both commands::optimize errors and std::fs::write errors,
ensuring the process exits non-zero on failure.
@madclaws madclaws merged commit 11a9a7f into tilesprivacy:main Jan 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate DSRs

2 participants