From dff50138ebba82d712be767e08e1c77f7ba4be7a Mon Sep 17 00:00:00 2001 From: Fabro Date: Thu, 19 Mar 2026 11:25:54 -0400 Subject: [PATCH] checkpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⚒️ Generated with [Fabro](https://fabro.sh) --- checkpoint.json | 48 ++++++++-- nodes/implement/prompt.md | 143 +++++++++++++++++++++++++++++ nodes/implement/provider_used.json | 5 + nodes/implement/response.md | 24 +++++ nodes/implement/status.json | 6 ++ 5 files changed, 219 insertions(+), 7 deletions(-) create mode 100644 nodes/implement/prompt.md create mode 100644 nodes/implement/provider_used.json create mode 100644 nodes/implement/response.md create mode 100644 nodes/implement/status.json diff --git a/checkpoint.json b/checkpoint.json index 243ad1d68..3beeaf51e 100644 --- a/checkpoint.json +++ b/checkpoint.json @@ -1,37 +1,44 @@ { - "timestamp": "2026-03-19T15:21:11.812777Z", - "current_node": "preflight_lint", + "timestamp": "2026-03-19T15:25:54.838170Z", + "current_node": "implement", "completed_nodes": [ "start", "toolchain", "preflight_compile", - "preflight_lint" + "preflight_lint", + "implement" ], "node_retries": { "start": 1, "preflight_lint": 1, + "implement": 1, "preflight_compile": 1, "toolchain": 1 }, "context_values": { "failure_class": "", "graph.goal": "# Fix: Default provider should respect configured API keys\n\n## Context\n\nUsers who only have OpenAI (or Gemini) configured hit an error when running workflows without an explicit `--provider` flag. The system hardcodes `Provider::Anthropic` as the fallback in several places, so it tries to call Anthropic even when no `ANTHROPIC_API_KEY` exists.\n\nThe fix: add a `Provider::default_from_env()` method that checks which providers have API keys and picks the best one, then replace all hardcoded `Provider::Anthropic` fallbacks with it.\n\n## Approach: Red/Green TDD\n\nEach step writes failing tests first, then implements to make them pass.\n\n---\n\n### Cycle 1: `Provider::default_with()` core logic\n\n**RED** — Add tests to `lib/crates/fabro-llm/src/provider.rs` (`mod tests`):\n\n```rust\n#[test]\nfn default_with_all_configured_prefers_anthropic() {\n assert_eq!(Provider::default_with(|_| true), Provider::Anthropic);\n}\n\n#[test]\nfn default_with_only_openai() {\n assert_eq!(Provider::default_with(|p| p == Provider::OpenAi), Provider::OpenAi);\n}\n\n#[test]\nfn default_with_only_gemini() {\n assert_eq!(Provider::default_with(|p| p == Provider::Gemini), Provider::Gemini);\n}\n\n#[test]\nfn default_with_openai_and_gemini_prefers_openai() {\n assert_eq!(\n Provider::default_with(|p| p == Provider::OpenAi || p == Provider::Gemini),\n Provider::OpenAi,\n );\n}\n\n#[test]\nfn default_with_none_configured_falls_back_to_anthropic() {\n assert_eq!(Provider::default_with(|_| false), Provider::Anthropic);\n}\n\n#[test]\nfn default_with_only_kimi_falls_back_to_anthropic() {\n assert_eq!(Provider::default_with(|p| p == Provider::Kimi), Provider::Anthropic);\n}\n```\n\nRun `cargo test -p fabro-llm` → compile error (method doesn't exist).\n\n**GREEN** — Add to `impl Provider` in the same file:\n\n```rust\n#[must_use]\npub fn default_from_env() -> Self {\n Self::default_with(Self::has_api_key)\n}\n\nfn default_with(is_configured: impl Fn(Self) -> bool) -> Self {\n const PRECEDENCE: [Provider; 3] = [Provider::Anthropic, Provider::OpenAi, Provider::Gemini];\n PRECEDENCE.iter().copied().find(|&p| is_configured(p)).unwrap_or(Provider::Anthropic)\n}\n```\n\nRun `cargo test -p fabro-llm` → all 6 new tests pass.\n\n---\n\n### Cycle 2: Replace hardcoded fallbacks\n\nThese are mechanical substitutions. For each site, the change is the same pattern: `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)`.\n\n**Sites to update:**\n\n| # | File | Line | What changes |\n|---|------|------|-------------|\n| 1 | `lib/crates/fabro-cli/src/commands/run.rs` | 211 | `resolve_model_provider()` provider fallback |\n| 2 | `lib/crates/fabro-cli/src/commands/run.rs` | 1123 | `run_command()` provider parse fallback |\n| 3 | `lib/crates/fabro-cli/src/commands/run.rs` | 1846-1853 | `run_from_branch()` — hardcoded `\"claude-opus-4-6\"` model + `Provider::Anthropic` |\n| 4 | `lib/crates/fabro-api/src/serve.rs` | 290-310 | `resolve_model_provider()` — `catalog::default_model()` + provider fallback |\n| 5 | `lib/crates/fabro-workflows/src/handler/prompt.rs` | 70 | prompt handler provider fallback |\n| 6 | `lib/crates/fabro-cli/src/commands/pr.rs` | 388 | `catalog::default_model()` → provider-aware default |\n\n**Special cases (not just unwrap_or swaps):**\n\n- **run.rs:1846** — replace `\"claude-opus-4-6\".to_string()` with catalog lookup using `default_from_env()`:\n ```rust\n let default_provider = Provider::default_from_env();\n let model = args.model.unwrap_or_else(|| {\n fabro_llm::catalog::default_model_for_provider(default_provider.as_str())\n .map(|m| m.id)\n .unwrap_or_else(|| default_provider.as_str().to_string())\n });\n ```\n\n- **serve.rs:294** — replace `catalog::default_model()` with provider-aware lookup:\n ```rust\n let default_provider = Provider::default_from_env();\n let default_info = provider_str\n .and_then(fabro_llm::catalog::default_model_for_provider)\n .unwrap_or_else(|| {\n fabro_llm::catalog::default_model_for_provider(default_provider.as_str())\n .unwrap_or_else(fabro_llm::catalog::default_model)\n });\n ```\n\n- **pr.rs:388** — replace `catalog::default_model()` with same pattern.\n\nRun `cargo test --workspace` after each file. Run `cargo clippy --workspace -- -D warnings` at the end.\n\n---\n\n## Verification\n\n1. `cargo fmt --check --all`\n2. `cargo clippy --workspace -- -D warnings`\n3. `cargo test --workspace`\n", - "current_node": "preflight_lint", + "current_node": "implement", "command.stderr": "", "command.output": "", "thread.preflight_compile.current_node": "preflight_lint", + "response.implement": "All checks pass:\n\n1. ✅ `cargo fmt --check --all` — no formatting issues\n2. ✅ `cargo clippy --workspace -- -D warnings` — no warnings\n3. ✅ `cargo test --workspace` — all tests pass (179 passed, 0 failed)\n\n## Summary of changes\n\n### Cycle 1: `Provider::default_with()` core logic (`lib/crates/fabro-llm/src/provider.rs`)\n\n- Added `Provider::default_from_env()` — public method that checks which providers have API keys and picks the best one (Anthropic → OpenAI → Gemini precedence, fallback to Anthropic)\n- Added `Provider::default_with()` — testable inner function that accepts a predicate instead of checking env vars directly\n- Added 6 unit tests covering all precedence/fallback scenarios\n\n### Cycle 2: Replace hardcoded fallbacks (6 sites)\n\n| # | File | Change |\n|---|------|--------|\n| 1 | `fabro-cli/src/commands/run.rs:211` | `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)` |\n| 2 | `fabro-cli/src/commands/run.rs:1175` | `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)` |\n| 3 | `fabro-cli/src/commands/run.rs:1898-1905` | Replaced hardcoded `\"claude-opus-4-6\"` with catalog lookup using `default_from_env()`, and provider fallback uses `default_from_env` |\n| 4 | `fabro-api/src/serve.rs:290-310` | Model resolution uses `default_from_env()` for provider-aware catalog lookup; provider fallback uses `default_from_env` |\n| 5 | `fabro-workflows/src/handler/prompt.rs:70` | `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)` |\n| 6 | `fabro-cli/src/commands/pr.rs:388` | `catalog::default_model()` → provider-aware lookup using `default_from_env()` |", + "thread.preflight_lint.current_node": "implement", "internal.node_visit_count": 1, + "internal.retry_count.implement": 1, "internal.retry_count.preflight_compile": 1, + "last_stage": "implement", "outcome": "success", "internal.retry_count.toolchain": 1, "thread.toolchain.current_node": "preflight_compile", - "current.preamble": "Goal: # Fix: Default provider should respect configured API keys\n\n## Context\n\nUsers who only have OpenAI (or Gemini) configured hit an error when running workflows without an explicit `--provider` flag. The system hardcodes `Provider::Anthropic` as the fallback in several places, so it tries to call Anthropic even when no `ANTHROPIC_API_KEY` exists.\n\nThe fix: add a `Provider::default_from_env()` method that checks which providers have API keys and picks the best one, then replace all hardcoded `Provider::Anthropic` fallbacks with it.\n\n## Approach: Red/Green TDD\n\nEach step writes failing tests first, then implements to make them pass.\n\n---\n\n### Cycle 1: `Provider::default_with()` core logic\n\n**RED** — Add tests to `lib/crates/fabro-llm/src/provider.rs` (`mod tests`):\n\n```rust\n#[test]\nfn default_with_all_configured_prefers_anthropic() {\n assert_eq!(Provider::default_with(|_| true), Provider::Anthropic);\n}\n\n#[test]\nfn default_with_only_openai() {\n assert_eq!(Provider::default_with(|p| p == Provider::OpenAi), Provider::OpenAi);\n}\n\n#[test]\nfn default_with_only_gemini() {\n assert_eq!(Provider::default_with(|p| p == Provider::Gemini), Provider::Gemini);\n}\n\n#[test]\nfn default_with_openai_and_gemini_prefers_openai() {\n assert_eq!(\n Provider::default_with(|p| p == Provider::OpenAi || p == Provider::Gemini),\n Provider::OpenAi,\n );\n}\n\n#[test]\nfn default_with_none_configured_falls_back_to_anthropic() {\n assert_eq!(Provider::default_with(|_| false), Provider::Anthropic);\n}\n\n#[test]\nfn default_with_only_kimi_falls_back_to_anthropic() {\n assert_eq!(Provider::default_with(|p| p == Provider::Kimi), Provider::Anthropic);\n}\n```\n\nRun `cargo test -p fabro-llm` → compile error (method doesn't exist).\n\n**GREEN** — Add to `impl Provider` in the same file:\n\n```rust\n#[must_use]\npub fn default_from_env() -> Self {\n Self::default_with(Self::has_api_key)\n}\n\nfn default_with(is_configured: impl Fn(Self) -> bool) -> Self {\n const PRECEDENCE: [Provider; 3] = [Provider::Anthropic, Provider::OpenAi, Provider::Gemini];\n PRECEDENCE.iter().copied().find(|&p| is_configured(p)).unwrap_or(Provider::Anthropic)\n}\n```\n\nRun `cargo test -p fabro-llm` → all 6 new tests pass.\n\n---\n\n### Cycle 2: Replace hardcoded fallbacks\n\nThese are mechanical substitutions. For each site, the change is the same pattern: `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)`.\n\n**Sites to update:**\n\n| # | File | Line | What changes |\n|---|------|------|-------------|\n| 1 | `lib/crates/fabro-cli/src/commands/run.rs` | 211 | `resolve_model_provider()` provider fallback |\n| 2 | `lib/crates/fabro-cli/src/commands/run.rs` | 1123 | `run_command()` provider parse fallback |\n| 3 | `lib/crates/fabro-cli/src/commands/run.rs` | 1846-1853 | `run_from_branch()` — hardcoded `\"claude-opus-4-6\"` model + `Provider::Anthropic` |\n| 4 | `lib/crates/fabro-api/src/serve.rs` | 290-310 | `resolve_model_provider()` — `catalog::default_model()` + provider fallback |\n| 5 | `lib/crates/fabro-workflows/src/handler/prompt.rs` | 70 | prompt handler provider fallback |\n| 6 | `lib/crates/fabro-cli/src/commands/pr.rs` | 388 | `catalog::default_model()` → provider-aware default |\n\n**Special cases (not just unwrap_or swaps):**\n\n- **run.rs:1846** — replace `\"claude-opus-4-6\".to_string()` with catalog lookup using `default_from_env()`:\n ```rust\n let default_provider = Provider::default_from_env();\n let model = args.model.unwrap_or_else(|| {\n fabro_llm::catalog::default_model_for_provider(default_provider.as_str())\n .map(|m| m.id)\n .unwrap_or_else(|| default_provider.as_str().to_string())\n });\n ```\n\n- **serve.rs:294** — replace `catalog::default_model()` with provider-aware lookup:\n ```rust\n let default_provider = Provider::default_from_env();\n let default_info = provider_str\n .and_then(fabro_llm::catalog::default_model_for_provider)\n .unwrap_or_else(|| {\n fabro_llm::catalog::default_model_for_provider(default_provider.as_str())\n .unwrap_or_else(fabro_llm::catalog::default_model)\n });\n ```\n\n- **pr.rs:388** — replace `catalog::default_model()` with same pattern.\n\nRun `cargo test --workspace` after each file. Run `cargo clippy --workspace -- -D warnings` at the end.\n\n---\n\n## Verification\n\n1. `cargo fmt --check --all`\n2. `cargo clippy --workspace -- -D warnings`\n3. `cargo test --workspace`\n\n\n## Completed stages\n- **toolchain**: success\n - Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`\n - Stdout:\n ```\n cargo 1.94.0 (85eff7c80 2026-01-15)\n ```\n - Stderr: (empty)\n- **preflight_compile**: success\n - Script: `cargo check -q --workspace 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n", + "current.preamble": "Goal: # Fix: Default provider should respect configured API keys\n\n## Context\n\nUsers who only have OpenAI (or Gemini) configured hit an error when running workflows without an explicit `--provider` flag. The system hardcodes `Provider::Anthropic` as the fallback in several places, so it tries to call Anthropic even when no `ANTHROPIC_API_KEY` exists.\n\nThe fix: add a `Provider::default_from_env()` method that checks which providers have API keys and picks the best one, then replace all hardcoded `Provider::Anthropic` fallbacks with it.\n\n## Approach: Red/Green TDD\n\nEach step writes failing tests first, then implements to make them pass.\n\n---\n\n### Cycle 1: `Provider::default_with()` core logic\n\n**RED** — Add tests to `lib/crates/fabro-llm/src/provider.rs` (`mod tests`):\n\n```rust\n#[test]\nfn default_with_all_configured_prefers_anthropic() {\n assert_eq!(Provider::default_with(|_| true), Provider::Anthropic);\n}\n\n#[test]\nfn default_with_only_openai() {\n assert_eq!(Provider::default_with(|p| p == Provider::OpenAi), Provider::OpenAi);\n}\n\n#[test]\nfn default_with_only_gemini() {\n assert_eq!(Provider::default_with(|p| p == Provider::Gemini), Provider::Gemini);\n}\n\n#[test]\nfn default_with_openai_and_gemini_prefers_openai() {\n assert_eq!(\n Provider::default_with(|p| p == Provider::OpenAi || p == Provider::Gemini),\n Provider::OpenAi,\n );\n}\n\n#[test]\nfn default_with_none_configured_falls_back_to_anthropic() {\n assert_eq!(Provider::default_with(|_| false), Provider::Anthropic);\n}\n\n#[test]\nfn default_with_only_kimi_falls_back_to_anthropic() {\n assert_eq!(Provider::default_with(|p| p == Provider::Kimi), Provider::Anthropic);\n}\n```\n\nRun `cargo test -p fabro-llm` → compile error (method doesn't exist).\n\n**GREEN** — Add to `impl Provider` in the same file:\n\n```rust\n#[must_use]\npub fn default_from_env() -> Self {\n Self::default_with(Self::has_api_key)\n}\n\nfn default_with(is_configured: impl Fn(Self) -> bool) -> Self {\n const PRECEDENCE: [Provider; 3] = [Provider::Anthropic, Provider::OpenAi, Provider::Gemini];\n PRECEDENCE.iter().copied().find(|&p| is_configured(p)).unwrap_or(Provider::Anthropic)\n}\n```\n\nRun `cargo test -p fabro-llm` → all 6 new tests pass.\n\n---\n\n### Cycle 2: Replace hardcoded fallbacks\n\nThese are mechanical substitutions. For each site, the change is the same pattern: `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)`.\n\n**Sites to update:**\n\n| # | File | Line | What changes |\n|---|------|------|-------------|\n| 1 | `lib/crates/fabro-cli/src/commands/run.rs` | 211 | `resolve_model_provider()` provider fallback |\n| 2 | `lib/crates/fabro-cli/src/commands/run.rs` | 1123 | `run_command()` provider parse fallback |\n| 3 | `lib/crates/fabro-cli/src/commands/run.rs` | 1846-1853 | `run_from_branch()` — hardcoded `\"claude-opus-4-6\"` model + `Provider::Anthropic` |\n| 4 | `lib/crates/fabro-api/src/serve.rs` | 290-310 | `resolve_model_provider()` — `catalog::default_model()` + provider fallback |\n| 5 | `lib/crates/fabro-workflows/src/handler/prompt.rs` | 70 | prompt handler provider fallback |\n| 6 | `lib/crates/fabro-cli/src/commands/pr.rs` | 388 | `catalog::default_model()` → provider-aware default |\n\n**Special cases (not just unwrap_or swaps):**\n\n- **run.rs:1846** — replace `\"claude-opus-4-6\".to_string()` with catalog lookup using `default_from_env()`:\n ```rust\n let default_provider = Provider::default_from_env();\n let model = args.model.unwrap_or_else(|| {\n fabro_llm::catalog::default_model_for_provider(default_provider.as_str())\n .map(|m| m.id)\n .unwrap_or_else(|| default_provider.as_str().to_string())\n });\n ```\n\n- **serve.rs:294** — replace `catalog::default_model()` with provider-aware lookup:\n ```rust\n let default_provider = Provider::default_from_env();\n let default_info = provider_str\n .and_then(fabro_llm::catalog::default_model_for_provider)\n .unwrap_or_else(|| {\n fabro_llm::catalog::default_model_for_provider(default_provider.as_str())\n .unwrap_or_else(fabro_llm::catalog::default_model)\n });\n ```\n\n- **pr.rs:388** — replace `catalog::default_model()` with same pattern.\n\nRun `cargo test --workspace` after each file. Run `cargo clippy --workspace -- -D warnings` at the end.\n\n---\n\n## Verification\n\n1. `cargo fmt --check --all`\n2. `cargo clippy --workspace -- -D warnings`\n3. `cargo test --workspace`\n\n\n## Completed stages\n- **toolchain**: success\n - Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`\n - Stdout:\n ```\n cargo 1.94.0 (85eff7c80 2026-01-15)\n ```\n - Stderr: (empty)\n- **preflight_compile**: success\n - Script: `cargo check -q --workspace 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n- **preflight_lint**: success\n - Script: `cargo clippy -q --workspace -- -D warnings 2>&1`\n - Stdout: (empty)\n - Stderr: (empty)\n", + "last_response": "All checks pass:\n\n1. ✅ `cargo fmt --check --all` — no formatting issues\n2. ✅ `cargo clippy --workspace -- -D warnings` — no warnings\n3. ✅ `cargo test --workspace` — all tests pass (179 pas", "internal.fidelity": "compact", "graph.model_stylesheet": "\n * { backend: api; model: claude-opus-4-6;}\n ", "internal.retry_count.start": 1, "internal.run_id": "01KM3AWHY48TDD4FB34M032P2R", "graph.rankdir": "LR", - "internal.thread_id": "preflight_compile", + "internal.thread_id": "preflight_lint", "failure_signature": "", "thread.start.current_node": "toolchain", "internal.retry_count.preflight_lint": 1 @@ -47,6 +54,32 @@ "notes": "Script completed: cargo clippy -q --workspace -- -D warnings 2>&1", "duration_ms": 13538 }, + "implement": { + "status": "success", + "context_updates": { + "last_stage": "implement", + "last_response": "All checks pass:\n\n1. ✅ `cargo fmt --check --all` — no formatting issues\n2. ✅ `cargo clippy --workspace -- -D warnings` — no warnings\n3. ✅ `cargo test --workspace` — all tests pass (179 pas", + "response.implement": "All checks pass:\n\n1. ✅ `cargo fmt --check --all` — no formatting issues\n2. ✅ `cargo clippy --workspace -- -D warnings` — no warnings\n3. ✅ `cargo test --workspace` — all tests pass (179 passed, 0 failed)\n\n## Summary of changes\n\n### Cycle 1: `Provider::default_with()` core logic (`lib/crates/fabro-llm/src/provider.rs`)\n\n- Added `Provider::default_from_env()` — public method that checks which providers have API keys and picks the best one (Anthropic → OpenAI → Gemini precedence, fallback to Anthropic)\n- Added `Provider::default_with()` — testable inner function that accepts a predicate instead of checking env vars directly\n- Added 6 unit tests covering all precedence/fallback scenarios\n\n### Cycle 2: Replace hardcoded fallbacks (6 sites)\n\n| # | File | Change |\n|---|------|--------|\n| 1 | `fabro-cli/src/commands/run.rs:211` | `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)` |\n| 2 | `fabro-cli/src/commands/run.rs:1175` | `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)` |\n| 3 | `fabro-cli/src/commands/run.rs:1898-1905` | Replaced hardcoded `\"claude-opus-4-6\"` with catalog lookup using `default_from_env()`, and provider fallback uses `default_from_env` |\n| 4 | `fabro-api/src/serve.rs:290-310` | Model resolution uses `default_from_env()` for provider-aware catalog lookup; provider fallback uses `default_from_env` |\n| 5 | `fabro-workflows/src/handler/prompt.rs:70` | `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)` |\n| 6 | `fabro-cli/src/commands/pr.rs:388` | `catalog::default_model()` → provider-aware lookup using `default_from_env()` |" + }, + "notes": "Stage completed: implement", + "usage": { + "model": "claude-opus-4-6", + "input_tokens": 22816, + "output_tokens": 5944, + "cache_read_tokens": 369792, + "cache_write_tokens": 34869, + "reasoning_tokens": 303, + "cost": 0.78804 + }, + "files_touched": [ + "/home/daytona/workspace/lib/crates/fabro-api/src/serve.rs", + "/home/daytona/workspace/lib/crates/fabro-cli/src/commands/pr.rs", + "/home/daytona/workspace/lib/crates/fabro-cli/src/commands/run.rs", + "/home/daytona/workspace/lib/crates/fabro-llm/src/provider.rs", + "/home/daytona/workspace/lib/crates/fabro-workflows/src/handler/prompt.rs" + ], + "duration_ms": 280400 + }, "start": { "status": "success", "duration_ms": 0 @@ -70,10 +103,11 @@ "duration_ms": 68137 } }, - "next_node_id": "implement", + "next_node_id": "simplify_opus", "node_visits": { "preflight_lint": 1, "start": 1, + "implement": 1, "toolchain": 1, "preflight_compile": 1 } diff --git a/nodes/implement/prompt.md b/nodes/implement/prompt.md new file mode 100644 index 000000000..99b9cb231 --- /dev/null +++ b/nodes/implement/prompt.md @@ -0,0 +1,143 @@ +Goal: # Fix: Default provider should respect configured API keys + +## Context + +Users who only have OpenAI (or Gemini) configured hit an error when running workflows without an explicit `--provider` flag. The system hardcodes `Provider::Anthropic` as the fallback in several places, so it tries to call Anthropic even when no `ANTHROPIC_API_KEY` exists. + +The fix: add a `Provider::default_from_env()` method that checks which providers have API keys and picks the best one, then replace all hardcoded `Provider::Anthropic` fallbacks with it. + +## Approach: Red/Green TDD + +Each step writes failing tests first, then implements to make them pass. + +--- + +### Cycle 1: `Provider::default_with()` core logic + +**RED** — Add tests to `lib/crates/fabro-llm/src/provider.rs` (`mod tests`): + +```rust +#[test] +fn default_with_all_configured_prefers_anthropic() { + assert_eq!(Provider::default_with(|_| true), Provider::Anthropic); +} + +#[test] +fn default_with_only_openai() { + assert_eq!(Provider::default_with(|p| p == Provider::OpenAi), Provider::OpenAi); +} + +#[test] +fn default_with_only_gemini() { + assert_eq!(Provider::default_with(|p| p == Provider::Gemini), Provider::Gemini); +} + +#[test] +fn default_with_openai_and_gemini_prefers_openai() { + assert_eq!( + Provider::default_with(|p| p == Provider::OpenAi || p == Provider::Gemini), + Provider::OpenAi, + ); +} + +#[test] +fn default_with_none_configured_falls_back_to_anthropic() { + assert_eq!(Provider::default_with(|_| false), Provider::Anthropic); +} + +#[test] +fn default_with_only_kimi_falls_back_to_anthropic() { + assert_eq!(Provider::default_with(|p| p == Provider::Kimi), Provider::Anthropic); +} +``` + +Run `cargo test -p fabro-llm` → compile error (method doesn't exist). + +**GREEN** — Add to `impl Provider` in the same file: + +```rust +#[must_use] +pub fn default_from_env() -> Self { + Self::default_with(Self::has_api_key) +} + +fn default_with(is_configured: impl Fn(Self) -> bool) -> Self { + const PRECEDENCE: [Provider; 3] = [Provider::Anthropic, Provider::OpenAi, Provider::Gemini]; + PRECEDENCE.iter().copied().find(|&p| is_configured(p)).unwrap_or(Provider::Anthropic) +} +``` + +Run `cargo test -p fabro-llm` → all 6 new tests pass. + +--- + +### Cycle 2: Replace hardcoded fallbacks + +These are mechanical substitutions. For each site, the change is the same pattern: `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)`. + +**Sites to update:** + +| # | File | Line | What changes | +|---|------|------|-------------| +| 1 | `lib/crates/fabro-cli/src/commands/run.rs` | 211 | `resolve_model_provider()` provider fallback | +| 2 | `lib/crates/fabro-cli/src/commands/run.rs` | 1123 | `run_command()` provider parse fallback | +| 3 | `lib/crates/fabro-cli/src/commands/run.rs` | 1846-1853 | `run_from_branch()` — hardcoded `"claude-opus-4-6"` model + `Provider::Anthropic` | +| 4 | `lib/crates/fabro-api/src/serve.rs` | 290-310 | `resolve_model_provider()` — `catalog::default_model()` + provider fallback | +| 5 | `lib/crates/fabro-workflows/src/handler/prompt.rs` | 70 | prompt handler provider fallback | +| 6 | `lib/crates/fabro-cli/src/commands/pr.rs` | 388 | `catalog::default_model()` → provider-aware default | + +**Special cases (not just unwrap_or swaps):** + +- **run.rs:1846** — replace `"claude-opus-4-6".to_string()` with catalog lookup using `default_from_env()`: + ```rust + let default_provider = Provider::default_from_env(); + let model = args.model.unwrap_or_else(|| { + fabro_llm::catalog::default_model_for_provider(default_provider.as_str()) + .map(|m| m.id) + .unwrap_or_else(|| default_provider.as_str().to_string()) + }); + ``` + +- **serve.rs:294** — replace `catalog::default_model()` with provider-aware lookup: + ```rust + let default_provider = Provider::default_from_env(); + let default_info = provider_str + .and_then(fabro_llm::catalog::default_model_for_provider) + .unwrap_or_else(|| { + fabro_llm::catalog::default_model_for_provider(default_provider.as_str()) + .unwrap_or_else(fabro_llm::catalog::default_model) + }); + ``` + +- **pr.rs:388** — replace `catalog::default_model()` with same pattern. + +Run `cargo test --workspace` after each file. Run `cargo clippy --workspace -- -D warnings` at the end. + +--- + +## Verification + +1. `cargo fmt --check --all` +2. `cargo clippy --workspace -- -D warnings` +3. `cargo test --workspace` + + +## Completed stages +- **toolchain**: success + - Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1` + - Stdout: + ``` + cargo 1.94.0 (85eff7c80 2026-01-15) + ``` + - Stderr: (empty) +- **preflight_compile**: success + - Script: `cargo check -q --workspace 2>&1` + - Stdout: (empty) + - Stderr: (empty) +- **preflight_lint**: success + - Script: `cargo clippy -q --workspace -- -D warnings 2>&1` + - Stdout: (empty) + - Stderr: (empty) + + +Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD. \ No newline at end of file diff --git a/nodes/implement/provider_used.json b/nodes/implement/provider_used.json new file mode 100644 index 000000000..c1905ff10 --- /dev/null +++ b/nodes/implement/provider_used.json @@ -0,0 +1,5 @@ +{ + "mode": "agent", + "provider": "anthropic", + "model": "claude-opus-4-6" +} \ No newline at end of file diff --git a/nodes/implement/response.md b/nodes/implement/response.md new file mode 100644 index 000000000..73a218d65 --- /dev/null +++ b/nodes/implement/response.md @@ -0,0 +1,24 @@ +All checks pass: + +1. ✅ `cargo fmt --check --all` — no formatting issues +2. ✅ `cargo clippy --workspace -- -D warnings` — no warnings +3. ✅ `cargo test --workspace` — all tests pass (179 passed, 0 failed) + +## Summary of changes + +### Cycle 1: `Provider::default_with()` core logic (`lib/crates/fabro-llm/src/provider.rs`) + +- Added `Provider::default_from_env()` — public method that checks which providers have API keys and picks the best one (Anthropic → OpenAI → Gemini precedence, fallback to Anthropic) +- Added `Provider::default_with()` — testable inner function that accepts a predicate instead of checking env vars directly +- Added 6 unit tests covering all precedence/fallback scenarios + +### Cycle 2: Replace hardcoded fallbacks (6 sites) + +| # | File | Change | +|---|------|--------| +| 1 | `fabro-cli/src/commands/run.rs:211` | `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)` | +| 2 | `fabro-cli/src/commands/run.rs:1175` | `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)` | +| 3 | `fabro-cli/src/commands/run.rs:1898-1905` | Replaced hardcoded `"claude-opus-4-6"` with catalog lookup using `default_from_env()`, and provider fallback uses `default_from_env` | +| 4 | `fabro-api/src/serve.rs:290-310` | Model resolution uses `default_from_env()` for provider-aware catalog lookup; provider fallback uses `default_from_env` | +| 5 | `fabro-workflows/src/handler/prompt.rs:70` | `.unwrap_or(Provider::Anthropic)` → `.unwrap_or_else(Provider::default_from_env)` | +| 6 | `fabro-cli/src/commands/pr.rs:388` | `catalog::default_model()` → provider-aware lookup using `default_from_env()` | \ No newline at end of file diff --git a/nodes/implement/status.json b/nodes/implement/status.json new file mode 100644 index 000000000..ab1f68b3d --- /dev/null +++ b/nodes/implement/status.json @@ -0,0 +1,6 @@ +{ + "status": "success", + "notes": "Stage completed: implement", + "failure_reason": null, + "timestamp": "2026-03-19T15:25:54.837646+00:00" +} \ No newline at end of file