fix: unbreak dev branch compilation after upstream/main merge#1
Open
Sigmachan wants to merge 1 commit into
Open
fix: unbreak dev branch compilation after upstream/main merge#1Sigmachan wants to merge 1 commit into
Sigmachan wants to merge 1 commit into
Conversation
The `dev` HEAD (merge of upstream/main) did not compile. This restores a green build and test suite without changing intended behavior. - session_control: bind `canonical_cwd` in `SessionStore::from_cwd` (referenced but never defined after the merge) - api/providers: cover `ProviderKind::Ollama` in `model_family_identity_for_kind` (Generic, matching the other OpenAI-compatible providers) - cli/main: close the unbalanced `else` block in `main()` error reporting - cli/main: restore the model-provenance subsystem the merge dropped (`ModelProvenance`, `ModelSource`; used by `status`) - cli/main: restore `stale_base_state_for` / `stale_base_json_value` (thin wrappers over the existing runtime `check_base_commit`) - cli/main: make `CliAction::HelpTopic` a struct variant carrying `output_format`, matching the parser and `print_help_topic` arity - cli/main: populate the `lifecycle` field when listing managed sessions via the existing `classify_session_lifecycle_for` - tests: `CARGO_BIN_EXE_claw` -> `CARGO_BIN_EXE_hackcode` after the binary rename Build: `cargo build --release` clean. Tests: 191 passing. Seven pre-existing failures remain (incomplete claw->hackcode help/banner strings, an mcp config error-capture gap, and a spinner-timing test) — all unrelated to compilation.
📝 WalkthroughWalkthroughThis PR makes targeted improvements across the API provider routing, runtime session management, CLI command routing, and status reporting. It fixes provider model-family classification, canonicalizes session paths, extends help-topic routing with output formats, optimizes session listing via lifecycle caching, introduces model-provenance types and stale-base JSON serialization, and updates all test harnesses to validate against the hackcode binary. ChangesCore CLI, API, and Runtime Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rust/crates/api/src/providers/mod.rs (1)
566-581:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd test coverage for Ollama mapping.
The test verifies provider-to-identity mapping for Anthropic, OpenAi, and Xai but omits Ollama. Adding coverage for the Ollama case will prevent regression and document the intended Generic mapping.
✅ Proposed test extension
#[test] fn maps_provider_kind_to_model_family_identity() { - // given: each supported provider kind + // given: all supported provider kinds let anthropic = ProviderKind::Anthropic; let openai = ProviderKind::OpenAi; let xai = ProviderKind::Xai; + let ollama = ProviderKind::Ollama; - // when: converting provider kinds to prompt model family identities + // when: converting provider kinds to model family identities let anthropic_identity = model_family_identity_for_kind(anthropic); let openai_identity = model_family_identity_for_kind(openai); let xai_identity = model_family_identity_for_kind(xai); + let ollama_identity = model_family_identity_for_kind(ollama); // then: Anthropic stays Claude and OpenAI-compatible providers are generic assert_eq!(anthropic_identity, runtime::ModelFamilyIdentity::Claude); assert_eq!(openai_identity, runtime::ModelFamilyIdentity::Generic); assert_eq!(xai_identity, runtime::ModelFamilyIdentity::Generic); + assert_eq!(ollama_identity, runtime::ModelFamilyIdentity::Generic); }🤖 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 `@rust/crates/api/src/providers/mod.rs` around lines 566 - 581, Extend the maps_provider_kind_to_model_family_identity test to include the Ollama provider: create let ollama = ProviderKind::Ollama, call model_family_identity_for_kind(ollama), and add an assertion that the result equals runtime::ModelFamilyIdentity::Generic; modify the existing test block (maps_provider_kind_to_model_family_identity) to include these lines so Ollama mapping coverage is present alongside Anthropic/OpenAi/Xai.rust/crates/rusty-claude-cli/src/main.rs (1)
6417-6424:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix model provenance resolution/source attribution in status output.
ModelProvenance::from_env_or_config_or_defaultnever emitsModelSource::Config, and on env hits it keepsresolvedas the caller-provided fallback model. This can emit inconsistent status payloads (model_source: "env"with a non-envmodel).Suggested patch
impl ModelProvenance { /// Probe the model-selection env vars; attribute to env when one is set, /// otherwise treat the resolved model as a built-in default. fn from_env_or_config_or_default(model: &str) -> Self { for key in ["HACKCODE_MODEL", "CLAW_MODEL", "ANTHROPIC_MODEL"] { if let Ok(value) = std::env::var(key) { let trimmed = value.trim(); if !trimmed.is_empty() { return Self { - resolved: model.to_string(), + resolved: resolve_model_alias_with_config(trimmed), raw: Some(trimmed.to_string()), source: ModelSource::Env, }; } } } + if let Some(config_model) = config_model_for_current_dir() { + let trimmed = config_model.trim(); + if !trimmed.is_empty() { + return Self { + resolved: resolve_model_alias_with_config(trimmed), + raw: Some(trimmed.to_string()), + source: ModelSource::Config, + }; + } + } Self { - resolved: model.to_string(), + resolved: resolve_model_alias_with_config(model), raw: None, source: ModelSource::Default, } } }Also applies to: 6481-6500
🤖 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 `@rust/crates/rusty-claude-cli/src/main.rs` around lines 6417 - 6424, The status output misattributes model provenance because ModelProvenance::from_env_or_config_or_default never sets ModelSource::Config and, on env hits, leaves resolved as the fallback; replace the blind call with logic that (a) checks for a flag (model_flag_raw) first (keep current branch), (b) otherwise explicitly attempts to resolve the model from config and env in order and sets ModelProvenance { resolved: <actual found value>, raw: None, source: ModelSource::Config|ModelSource::Env } (or ModelSource::Default if neither found). Update or add a helper (e.g., ModelProvenance::resolve_from_config_or_env_or_default) and use it where the old call appears (the provenance variable here and the similar block around the 6481–6500 region) so resolved and source are consistent.
🤖 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.
Outside diff comments:
In `@rust/crates/api/src/providers/mod.rs`:
- Around line 566-581: Extend the maps_provider_kind_to_model_family_identity
test to include the Ollama provider: create let ollama = ProviderKind::Ollama,
call model_family_identity_for_kind(ollama), and add an assertion that the
result equals runtime::ModelFamilyIdentity::Generic; modify the existing test
block (maps_provider_kind_to_model_family_identity) to include these lines so
Ollama mapping coverage is present alongside Anthropic/OpenAi/Xai.
In `@rust/crates/rusty-claude-cli/src/main.rs`:
- Around line 6417-6424: The status output misattributes model provenance
because ModelProvenance::from_env_or_config_or_default never sets
ModelSource::Config and, on env hits, leaves resolved as the fallback; replace
the blind call with logic that (a) checks for a flag (model_flag_raw) first
(keep current branch), (b) otherwise explicitly attempts to resolve the model
from config and env in order and sets ModelProvenance { resolved: <actual found
value>, raw: None, source: ModelSource::Config|ModelSource::Env } (or
ModelSource::Default if neither found). Update or add a helper (e.g.,
ModelProvenance::resolve_from_config_or_env_or_default) and use it where the old
call appears (the provenance variable here and the similar block around the
6481–6500 region) so resolved and source are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ab3b56a-5872-4ec4-9ee1-070949f49757
📒 Files selected for processing (9)
rust/crates/api/src/providers/mod.rsrust/crates/runtime/src/session_control.rsrust/crates/rusty-claude-cli/src/main.rsrust/crates/rusty-claude-cli/tests/cli_flags_and_config_defaults.rsrust/crates/rusty-claude-cli/tests/compact_output.rsrust/crates/rusty-claude-cli/tests/compact_repl_panic.rsrust/crates/rusty-claude-cli/tests/mock_parity_harness.rsrust/crates/rusty-claude-cli/tests/output_format_contract.rsrust/crates/rusty-claude-cli/tests/resume_slash_commands.rs
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.
What
The
devHEAD (theMerge remote-tracking branch 'upstream/main' into devcommit) does not compile —cargo build --releasefails before producing thehackcodebinary. This PR restores a green build and test suite without changing intended behavior: every change either re-binds a symbol the merge left dangling, restores a subsystem the merge dropped, or repairs a syntactic/arity mismatch.Fixes
Build blockers (
cargo build --release):runtime/session_control.rs—SessionStore::from_cwdreferencedcanonical_cwd, which was never bound. Canonicalizecwdthe same wayfrom_data_dirdoes.api/providers/mod.rs—model_family_identity_for_kinddid not coverProviderKind::Ollama(non-exhaustive match). Mapped toGeneric, alongside the other OpenAI-compatible providers.rusty-claude-cli/main.rs— unbalanced delimiters: the finalelseblock inmain()'s error reporting was never closed.rusty-claude-cli/main.rs— restored the model-provenance types the merge dropped (ModelProvenance,ModelSource) used by thestatuscommand (#148).rusty-claude-cli/main.rs— restoredstale_base_state_for/stale_base_json_value(#148) as thin wrappers over the existingruntime::{resolve_expected_base, check_base_commit}.rusty-claude-cli/main.rs—CliAction::HelpTopicis now a struct variant{ topic, output_format }, matching its parser construction site andprint_help_topic's two-argument signature.rusty-claude-cli/main.rs— populate thelifecyclefield when collecting managed sessions, via the existingclassify_session_lifecycle_for(restores thesaved only · dirty worktree · abandoned?signal).Test build blocker (
cargo test):rusty-claude-cli/tests/*.rs—env!("CARGO_BIN_EXE_claw")→CARGO_BIN_EXE_hackcodeafter the binary was renamed tohackcode(Cargo derives the env var from the[[bin]] name).Verification
cargo build --release— clean.cargo test -p rusty-claude-cli— 191 passing.claw→hackcodestrings in help/banner text (init_help_mentions_direct_subcommand,help_mentions_jsonl_resume_examples,startup_banner_mentions_workflow_completions), an MCP-config error-capture gap (status_degrades_gracefully_on_malformed_mcp_config_143), a boot-preflight contract test, an init artifacts test, and a spinner-timing test (render::spinner_advances_frames). Happy to fold fixes for any of these into a follow-up if useful.Summary by CodeRabbit
Bug Fixes
New Features
Chores