mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-10 03:30:59 +00:00
parent
54a43c567d
commit
87aed8109f
4 changed files with 38 additions and 8 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"timestamp": "2026-03-16T04:06:36.368668Z",
|
||||
"current_node": "simplify_gpt",
|
||||
"timestamp": "2026-03-16T04:07:55.288346Z",
|
||||
"current_node": "verify",
|
||||
"completed_nodes": [
|
||||
"start",
|
||||
"toolchain",
|
||||
|
|
@ -9,20 +9,22 @@
|
|||
"implement",
|
||||
"simplify_opus",
|
||||
"simplify_gemini",
|
||||
"simplify_gpt"
|
||||
"simplify_gpt",
|
||||
"verify"
|
||||
],
|
||||
"node_retries": {
|
||||
"simplify_gemini": 1,
|
||||
"implement": 1,
|
||||
"preflight_compile": 1,
|
||||
"simplify_gpt": 1,
|
||||
"verify": 1,
|
||||
"simplify_opus": 1,
|
||||
"start": 1,
|
||||
"toolchain": 1,
|
||||
"preflight_lint": 1
|
||||
},
|
||||
"context_values": {
|
||||
"current_node": "simplify_gpt",
|
||||
"current_node": "verify",
|
||||
"response.simplify_gemini": "All checks pass. Let me summarize the review:\n\n## Review Summary\n\n**All three review agents completed. Aggregated findings:**\n\n### Code Reuse Review\n- No new functions or duplicate logic were introduced. The refactor purely collapses existing fields into an `Option<PullRequestConfig>`, replacing `pull_request_enabled: false, pull_request_draft: false` with `pull_request: None` everywhere.\n- `AutoMergeConfig` is already gone (confirmed via grep — zero matches).\n- No old flat fields (`pull_request_enabled`, `pull_request_draft`) remain anywhere.\n\n### Code Quality Review \n- **`maybe_open_pull_request` still takes `draft: bool`** — The goal mentioned changing this to take `&PullRequestConfig`, but keeping `draft: bool` is the right call here. The `pr.rs` CLI command (`fabro pr create`) hardcodes `draft: true` and doesn't have a `PullRequestConfig` object. Changing the function to require `&PullRequestConfig` would force creating a throwaway config struct. The `#[allow(clippy::too_many_arguments)]` is already present. **Not an issue — skip.**\n- **`enabled` field on `PullRequestConfig` is redundant at runtime** — This is by design. The field is needed for TOML deserialization (`enabled = true`), and the construction in `run.rs` correctly filters it out with `.filter(|p| p.enabled).cloned()`. **Not an issue — working as intended.**\n- **Nesting depth in `run.rs`** — The `if let Some(ref pr_cfg)` adds one level of nesting, but the overall structure is clearer: the entire PR block is scoped under the Option check, with a clean `else` branch for the debug log. **Acceptable.**\n\n### Efficiency Review\n- **`.cloned()` on `PullRequestConfig`** — `PullRequestConfig` has only 2 `bool` fields (2 bytes). Clone cost is negligible. **Not an issue.**\n- No unnecessary allocations, redundant computations, or TOCTOU patterns introduced.\n\n### Verification\n- `cargo build --workspace` ✅\n- `cargo clippy --workspace -- -D warnings` ✅ \n- `cargo test -p fabro-workflows -- pull_request run_config project_config` ✅ (1 passed)\n- `cargo fmt --check --all` ✅\n\n**Conclusion: The code is clean. No fixes needed.**",
|
||||
"internal.retry_count.simplify_opus": 1,
|
||||
"internal.retry_count.simplify_gpt": 1,
|
||||
|
|
@ -37,9 +39,11 @@
|
|||
"thread.implement.current_node": "simplify_opus",
|
||||
"thread.start.current_node": "toolchain",
|
||||
"graph.goal": "# Refactor: Collapse `pull_request_*` fields on `RunConfig` into `Option<PullRequestConfig>`\n\n## Context\n\n`RunConfig` has 4 flat `pull_request_*` fields that encode a natural tree structure. This refactor collapses them into a single `Option<PullRequestConfig>` field, reusing the existing TOML config struct directly.\n\n## Approach\n\nReplace on `RunConfig`:\n```rust\npub pull_request_enabled: bool,\npub pull_request_draft: bool,\npub pull_request_auto_merge: bool,\npub pull_request_merge_strategy: MergeStrategy,\n```\n\nWith:\n```rust\npub pull_request: Option<PullRequestConfig>,\n```\n\n- `None` = disabled (replaces `enabled: false`)\n- `Some(config)` = enabled, read `.draft`, `.auto_merge`, `.merge_strategy` directly\n- The `enabled` field on `PullRequestConfig` is still needed for TOML deserialization but is redundant at runtime\n\nDelete `AutoMergeConfig` from `pull_request.rs` — pass `Option<MergeStrategy>` directly instead (derived from `auto_merge` + `merge_strategy` on `PullRequestConfig`).\n\n## Files to modify\n\n1. **`engine.rs`** — replace 4 fields with `pull_request: Option<PullRequestConfig>`, update all test constructors (4 lines → 1: `pull_request: None`)\n2. **`run.rs`** — simplify construction: `pull_request: pr_cfg.filter(|p| p.enabled).cloned()` with draft override for auto_merge. Simplify read site: `if let Some(ref pr) = config.pull_request { ... }`\n3. **`pull_request.rs`** — change `maybe_open_pull_request` to take `&PullRequestConfig` instead of `draft: bool` + `auto_merge: Option<AutoMergeConfig>`. Delete `AutoMergeConfig`\n4. **`pr.rs`** — update call to `maybe_open_pull_request`\n5. **`server.rs`, `manager_loop.rs`** — `pull_request: None`\n6. **`tests/integration.rs`, `tests/daytona_integration.rs`** — `pull_request: None`\n\n## Verification\n\n- `cargo build --workspace`\n- `cargo clippy --workspace -- -D warnings`\n- `cargo test -p fabro-workflows -- pull_request run_config project_config`\n- `cargo fmt --check --all`\n",
|
||||
"internal.thread_id": "simplify_gemini",
|
||||
"internal.thread_id": "simplify_gpt",
|
||||
"internal.retry_count.verify": 1,
|
||||
"thread.simplify_gemini.current_node": "simplify_gpt",
|
||||
"internal.fidelity": "compact",
|
||||
"thread.simplify_gpt.current_node": "verify",
|
||||
"thread.preflight_lint.current_node": "implement",
|
||||
"failure_class": "",
|
||||
"last_response": "Good — it doesn't exist (was never present, the goal doc mentioned it as a future step but it wasn't created in the first place).\n\n## Summary\n\nThe code is already clean. After reviewing all changes ",
|
||||
|
|
@ -48,14 +52,14 @@
|
|||
"outcome": "success",
|
||||
"response.simplify_gpt": "Good — it doesn't exist (was never present, the goal doc mentioned it as a future step but it wasn't created in the first place).\n\n## Summary\n\nThe code is already clean. After reviewing all changes across the three dimensions:\n\n**Code Reuse:** No issues found. The `.and_then().filter().cloned()` chain is idiomatic Rust and doesn't duplicate any existing utility. The pattern is simple enough not to warrant extraction.\n\n**Code Quality:** No issues found. The refactoring is clean:\n- `None` = disabled, `Some(config)` = enabled — this correctly eliminates the redundant boolean at the `RunConfig` level\n- The `enabled` field on `PullRequestConfig` is retained only for TOML deserialization (as designed), and filtered out via `.filter(|p| p.enabled)` during construction\n- The `maybe_open_pull_request` function correctly keeps `draft: bool` as a parameter since it's also called from `pr.rs` where no `PullRequestConfig` is available\n- The restructured `if let Some(ref pr_cfg)` block in `run.rs` is cleaner than the original flat if-else chain\n\n**Efficiency:** No issues found. The `.cloned()` on `PullRequestConfig` (which is just `bool` + `bool`) is negligible. No new file reads, no redundant work, no hot-path bloat.\n\nAll verification checks pass: build ✅, clippy ✅, tests ✅, fmt ✅.",
|
||||
"last_stage": "simplify_gpt",
|
||||
"current.preamble": "Goal: # Refactor: Collapse `pull_request_*` fields on `RunConfig` into `Option<PullRequestConfig>`\n\n## Context\n\n`RunConfig` has 4 flat `pull_request_*` fields that encode a natural tree structure. This refactor collapses them into a single `Option<PullRequestConfig>` field, reusing the existing TOML config struct directly.\n\n## Approach\n\nReplace on `RunConfig`:\n```rust\npub pull_request_enabled: bool,\npub pull_request_draft: bool,\npub pull_request_auto_merge: bool,\npub pull_request_merge_strategy: MergeStrategy,\n```\n\nWith:\n```rust\npub pull_request: Option<PullRequestConfig>,\n```\n\n- `None` = disabled (replaces `enabled: false`)\n- `Some(config)` = enabled, read `.draft`, `.auto_merge`, `.merge_strategy` directly\n- The `enabled` field on `PullRequestConfig` is still needed for TOML deserialization but is redundant at runtime\n\nDelete `AutoMergeConfig` from `pull_request.rs` — pass `Option<MergeStrategy>` directly instead (derived from `auto_merge` + `merge_strategy` on `PullRequestConfig`).\n\n## Files to modify\n\n1. **`engine.rs`** — replace 4 fields with `pull_request: Option<PullRequestConfig>`, update all test constructors (4 lines → 1: `pull_request: None`)\n2. **`run.rs`** — simplify construction: `pull_request: pr_cfg.filter(|p| p.enabled).cloned()` with draft override for auto_merge. Simplify read site: `if let Some(ref pr) = config.pull_request { ... }`\n3. **`pull_request.rs`** — change `maybe_open_pull_request` to take `&PullRequestConfig` instead of `draft: bool` + `auto_merge: Option<AutoMergeConfig>`. Delete `AutoMergeConfig`\n4. **`pr.rs`** — update call to `maybe_open_pull_request`\n5. **`server.rs`, `manager_loop.rs`** — `pull_request: None`\n6. **`tests/integration.rs`, `tests/daytona_integration.rs`** — `pull_request: None`\n\n## Verification\n\n- `cargo build --workspace`\n- `cargo clippy --workspace -- -D warnings`\n- `cargo test -p fabro-workflows -- pull_request run_config project_config`\n- `cargo fmt --check --all`\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- **implement**: success\n - Model: claude-opus-4-6, 56.0k tokens in / 11.6k out\n - Files: /home/daytona/workspace/lib/crates/fabro-api/src/server.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/pr.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/handler/manager_loop.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/pull_request.rs\n- **simplify_opus**: success\n - Model: claude-opus-4-6, 66.9k tokens in / 8.3k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/pr.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/pull_request.rs\n- **simplify_gemini**: success\n - Model: claude-opus-4-6, 81.4k tokens in / 6.9k out\n",
|
||||
"current.preamble": "Goal: # Refactor: Collapse `pull_request_*` fields on `RunConfig` into `Option<PullRequestConfig>`\n\n## Context\n\n`RunConfig` has 4 flat `pull_request_*` fields that encode a natural tree structure. This refactor collapses them into a single `Option<PullRequestConfig>` field, reusing the existing TOML config struct directly.\n\n## Approach\n\nReplace on `RunConfig`:\n```rust\npub pull_request_enabled: bool,\npub pull_request_draft: bool,\npub pull_request_auto_merge: bool,\npub pull_request_merge_strategy: MergeStrategy,\n```\n\nWith:\n```rust\npub pull_request: Option<PullRequestConfig>,\n```\n\n- `None` = disabled (replaces `enabled: false`)\n- `Some(config)` = enabled, read `.draft`, `.auto_merge`, `.merge_strategy` directly\n- The `enabled` field on `PullRequestConfig` is still needed for TOML deserialization but is redundant at runtime\n\nDelete `AutoMergeConfig` from `pull_request.rs` — pass `Option<MergeStrategy>` directly instead (derived from `auto_merge` + `merge_strategy` on `PullRequestConfig`).\n\n## Files to modify\n\n1. **`engine.rs`** — replace 4 fields with `pull_request: Option<PullRequestConfig>`, update all test constructors (4 lines → 1: `pull_request: None`)\n2. **`run.rs`** — simplify construction: `pull_request: pr_cfg.filter(|p| p.enabled).cloned()` with draft override for auto_merge. Simplify read site: `if let Some(ref pr) = config.pull_request { ... }`\n3. **`pull_request.rs`** — change `maybe_open_pull_request` to take `&PullRequestConfig` instead of `draft: bool` + `auto_merge: Option<AutoMergeConfig>`. Delete `AutoMergeConfig`\n4. **`pr.rs`** — update call to `maybe_open_pull_request`\n5. **`server.rs`, `manager_loop.rs`** — `pull_request: None`\n6. **`tests/integration.rs`, `tests/daytona_integration.rs`** — `pull_request: None`\n\n## Verification\n\n- `cargo build --workspace`\n- `cargo clippy --workspace -- -D warnings`\n- `cargo test -p fabro-workflows -- pull_request run_config project_config`\n- `cargo fmt --check --all`\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- **implement**: success\n - Model: claude-opus-4-6, 56.0k tokens in / 11.6k out\n - Files: /home/daytona/workspace/lib/crates/fabro-api/src/server.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/pr.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/handler/manager_loop.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/pull_request.rs\n- **simplify_opus**: success\n - Model: claude-opus-4-6, 66.9k tokens in / 8.3k out\n - Files: /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/pr.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/cli/run.rs, /home/daytona/workspace/lib/crates/fabro-workflows/src/pull_request.rs\n- **simplify_gemini**: success\n - Model: claude-opus-4-6, 81.4k tokens in / 6.9k out\n- **simplify_gpt**: success\n - Model: claude-opus-4-6, 76.8k tokens in / 5.3k out\n",
|
||||
"internal.retry_count.implement": 1,
|
||||
"internal.run_id": "01KKTC2TWVRW6ZCYYZYGRX0GD6",
|
||||
"graph.model_stylesheet": "\n * { backend: api; model: claude-opus-4-6;}\n ",
|
||||
"internal.node_visit_count": 1,
|
||||
"internal.retry_count.start": 1,
|
||||
"internal.retry_count.preflight_lint": 1,
|
||||
"command.output": "",
|
||||
"command.output": "────────────\n Nextest run ID 05dd2c88-c871-44d4-987d-7f7f251ff246 with nextest profile: default\n Starting 3409 tests across 38 binaries (183 tests skipped)\n────────────\n Summary [ 14.195s] 3409 tests run: 3409 passed, 183 skipped\n",
|
||||
"thread.toolchain.current_node": "preflight_compile"
|
||||
},
|
||||
"logs": [],
|
||||
|
|
@ -132,6 +136,15 @@
|
|||
"status": "success",
|
||||
"duration_ms": 0
|
||||
},
|
||||
"verify": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
"command.output": "────────────\n Nextest run ID 05dd2c88-c871-44d4-987d-7f7f251ff246 with nextest profile: default\n Starting 3409 tests across 38 binaries (183 tests skipped)\n────────────\n Summary [ 14.195s] 3409 tests run: 3409 passed, 183 skipped\n",
|
||||
"command.stderr": ""
|
||||
},
|
||||
"notes": "Script completed: cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1",
|
||||
"duration_ms": 76675
|
||||
},
|
||||
"simplify_gpt": {
|
||||
"status": "success",
|
||||
"context_updates": {
|
||||
|
|
@ -180,11 +193,12 @@
|
|||
"duration_ms": 68455
|
||||
}
|
||||
},
|
||||
"next_node_id": "verify",
|
||||
"next_node_id": "fmt",
|
||||
"node_visits": {
|
||||
"preflight_compile": 1,
|
||||
"start": 1,
|
||||
"toolchain": 1,
|
||||
"verify": 1,
|
||||
"implement": 1,
|
||||
"simplify_gpt": 1,
|
||||
"simplify_gemini": 1,
|
||||
|
|
|
|||
5
nodes/verify/script_invocation.json
Normal file
5
nodes/verify/script_invocation.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"command": "cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1",
|
||||
"language": "shell",
|
||||
"timeout_ms": null
|
||||
}
|
||||
5
nodes/verify/script_timing.json
Normal file
5
nodes/verify/script_timing.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"duration_ms": 76672,
|
||||
"exit_code": 0,
|
||||
"timed_out": false
|
||||
}
|
||||
6
nodes/verify/status.json
Normal file
6
nodes/verify/status.json
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"status": "success",
|
||||
"notes": "Script completed: cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1",
|
||||
"failure_reason": null,
|
||||
"timestamp": "2026-03-16T04:07:55.286784+00:00"
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue