Chore/refactor structure#81
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe project is reorganized into a multi-crate Cargo workspace. Shared ownership and IR types move into ChangesMulti-crate workspace extraction
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
artefactop
commented
Jul 2, 2026
artefactop
commented
Jul 2, 2026
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
artefactop
commented
Jul 2, 2026
artefactop
commented
Jul 2, 2026
artefactop
commented
Jul 2, 2026
| use crate::types::InternPool; | ||
| use crate::uir::Uir; | ||
| #[derive(Clone, Copy, Debug, PartialEq, Eq, clap::ValueEnum)] | ||
| pub enum EmitKind { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/superpowers/specs/2026-07-02-cargo-workspace-refactor-design.md (1)
1-217: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename this spec and retag the examples.
docs/superpowers/specs/2026-07-02-cargo-workspace-refactor-design.mdviolates the docs filename rule, and the fenced examples below usetoml/rust/bashinstead of the requiredryolanguage tag. Please rename it to use underscores and switch every code block toryo. As per coding guidelines, documentation files must use lowercase with underscores naming convention except for special files like README.md, CLAUDE.md, and TODO.md, and code examples in documentation must use fenced code blocks withryolanguage tag.🤖 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 `@docs/superpowers/specs/2026-07-02-cargo-workspace-refactor-design.md` around lines 1 - 217, The spec file name needs to conform to the docs naming rule, and all fenced examples in this spec must use the required ryo language tag. Rename the document to use lowercase underscores instead of hyphens, then update every fenced code block in the design sections (for example, the Cargo manifest snippets) so they are tagged with ryo rather than toml, rust, or bash. Focus on the examples and markup in this spec document only.Source: Coding guidelines
🧹 Nitpick comments (1)
ryo/build.rs (1)
3-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
root_dirfor the git paths instead of hardcoded"../".
root_diris computed later at Line 21 and correctly used for the runtime target dir andruntime_src, but the git-related paths at Lines 6-8 and 100 still hardcode"../.git/...". Both represent the same "workspace root" concept; keeping one path derivation would avoid the two silently diverging if the crate is ever nested differently again.♻️ Proposed consolidation
fn main() { - println!("cargo:rerun-if-changed=../.git/HEAD"); - if let Some(git_ref) = resolve_git_ref() { - println!("cargo:rerun-if-changed=../{git_ref}"); - } + let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); + let root_dir = PathBuf::from(&manifest_dir).parent().unwrap().to_path_buf(); + + println!("cargo:rerun-if-changed={}", root_dir.join(".git/HEAD").display()); + if let Some(git_ref) = resolve_git_ref(&root_dir) { + println!("cargo:rerun-if-changed={}", root_dir.join(git_ref).display()); + } let pkg_version = env::var("CARGO_PKG_VERSION").unwrap_or_else(|_| "0.0.0".to_string()); let short_hash = get_git_short_hash(); let commit_date = get_git_commit_date(); let version = match (short_hash, commit_date) { (Some(hash), Some(date)) => format!("{pkg_version}-dev.{date}+{hash}"), (Some(hash), None) => format!("{pkg_version}-dev+{hash}"), _ => pkg_version, }; println!("cargo:rustc-env=RYO_VERSION={version}"); - let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); - let root_dir = PathBuf::from(&manifest_dir).parent().unwrap().to_path_buf(); - // Runtime archive path. ...-fn resolve_git_ref() -> Option<String> { - let head = std::fs::read_to_string("../.git/HEAD").ok()?; +fn resolve_git_ref(root_dir: &std::path::Path) -> Option<String> { + let head = std::fs::read_to_string(root_dir.join(".git/HEAD")).ok()?; let head = head.trim(); head.strip_prefix("ref: ") .map(|refpath| format!(".git/{refpath}")) }Also applies to: 20-21, 99-103
🤖 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 `@ryo/build.rs` around lines 3 - 8, The git path handling in main() still hardcodes "../" even though root_dir already represents the workspace root; update the git-related cargo:rerun-if-changed paths to derive from root_dir instead of string literals. Reuse the same root_dir-based path construction used for the runtime target dir and runtime_src so resolve_git_ref and the later git path logic stay consistent if the crate layout changes.
🤖 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 `@docs/superpowers/plans/2026-07-02-cargo-workspace-refactor.md`:
- Around line 1-561: The plan doc needs to follow the docs filename convention
and all fenced examples must use the required language tag. Rename the document
to use lowercase underscores instead of hyphens, and update every code fence in
this file from toml/rust/bash to ryo; use the unique symbols in the plan itself,
such as the repeated fenced examples and task sections, to verify all blocks
were retagged consistently.
---
Outside diff comments:
In `@docs/superpowers/specs/2026-07-02-cargo-workspace-refactor-design.md`:
- Around line 1-217: The spec file name needs to conform to the docs naming
rule, and all fenced examples in this spec must use the required ryo language
tag. Rename the document to use lowercase underscores instead of hyphens, then
update every fenced code block in the design sections (for example, the Cargo
manifest snippets) so they are tagged with ryo rather than toml, rust, or bash.
Focus on the examples and markup in this spec document only.
---
Nitpick comments:
In `@ryo/build.rs`:
- Around line 3-8: The git path handling in main() still hardcodes "../" even
though root_dir already represents the workspace root; update the git-related
cargo:rerun-if-changed paths to derive from root_dir instead of string literals.
Reuse the same root_dir-based path construction used for the runtime target dir
and runtime_src so resolve_git_ref and the later git path logic stay consistent
if the crate layout changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c12a9ca2-475b-431f-bccd-51b1a93da389
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
Cargo.tomldocs/superpowers/plans/2026-07-02-cargo-workspace-refactor.mddocs/superpowers/specs/2026-07-02-cargo-workspace-refactor-design.mdryo-backend/Cargo.tomlryo-backend/build.rsryo-backend/src/codegen.rsryo-backend/src/lib.rsryo-backend/src/linker.rsryo-backend/src/runtime_lib.rsryo-backend/src/toolchain.rsryo-core/Cargo.tomlryo-core/src/ast.rsryo-core/src/diag.rsryo-core/src/errors.rsryo-core/src/lib.rsryo-core/src/ownership.rsryo-core/src/tir.rsryo-core/src/types.rsryo-core/src/uir.rsryo-driver/Cargo.tomlryo-driver/src/lib.rsryo-driver/src/pipeline.rsryo-frontend/Cargo.tomlryo-frontend/src/astgen.rsryo-frontend/src/builtins.rsryo-frontend/src/indent.rsryo-frontend/src/lexer.rsryo-frontend/src/lib.rsryo-frontend/src/ownership.rsryo-frontend/src/parser.rsryo-frontend/src/sema.rsryo/Cargo.tomlryo/build.rsryo/src/main.rsryo/tests/asan_smoke.rsryo/tests/common/mod.rsryo/tests/integration_tests.rsryo/tests/valgrind_smoke.rssrc/CLAUDE.md
💤 Files with no reviewable changes (1)
- src/CLAUDE.md
artefactop
commented
Jul 2, 2026
…ementation roadmap
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@CLAUDE.md`:
- Around line 8-17: The workspace section in CLAUDE.md conflates actual Cargo
workspace members with general repository directories. Update the layout
description to separate the real workspace members (such as ryo, ryo-core,
ryo-frontend, ryo-backend, ryo-driver, and runtime) from non-member repository
areas like docs, experimental, and .github, and keep the repository layout
wording aligned with the root manifest so contributors are not misled.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c579589a-2b0e-4f05-a7fa-a0c549848efc
📒 Files selected for processing (11)
CLAUDE.mddocs/dev/CLAUDE.mddocs/dev/arc_optimizer.mddocs/dev/implementation_roadmap.mddocs/dev/installation.mddocs/dev/mojo_reference.mddocs/dev/pipeline_alignment.mddocs/dev/project_structure.mddocs/dev/proposals/wasm_target.mddocs/dev/rust_reference.mdryo/build.rs
💤 Files with no reviewable changes (1)
- docs/dev/project_structure.md
✅ Files skipped from review due to trivial changes (8)
- docs/dev/installation.md
- docs/dev/rust_reference.md
- docs/dev/implementation_roadmap.md
- docs/dev/mojo_reference.md
- docs/dev/CLAUDE.md
- docs/dev/arc_optimizer.md
- docs/dev/proposals/wasm_target.md
- docs/dev/pipeline_alignment.md
🚧 Files skipped from review as they are similar to previous changes (1)
- ryo/build.rs
The layout section conflated actual Cargo workspace members with general repository directories. Group the 6 manifest members separately from non-member areas (docs/, .github/), and drop the stale experimental/ entry, which no longer exists in the repo.
- ci: test/clippy/fmt now run across the full workspace (cargo test --workspace, clippy --workspace --all-targets, fmt --all) Restores ~333 unit tests in ryo-frontend/ryo-core/ryo-backend that silently stopped running after default-members=[ryo]. - ci: fix ZIG_VERSION grep path src/toolchain.rs -> ryo-backend/src/toolchain.rs - build(ryo-backend): TODO noting build.rs runtime-resolution is duplicated verbatim from ryo/build.rs (extract into shared crate) - docs: fix stale src/arc_optimizer.rs ref in CLAUDE.md - deps(ryo-frontend): drop unused hashbrown - docs: refresh stale src/*.rs paths in active docs to workspace paths
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 by CodeRabbit