Expose source annotations and canonicalize the frontend - #26
Conversation
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThe change replaces version-specific syntax and checking APIs with canonical interfaces, adds structured UI markup annotations and authoring projections, integrates them into provenance and editor rendering, and strengthens CLI/CI coverage for authoring, previews, and provenance. ChangesCanonical Uhura frontend
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/uhura-syntax/src/ui.rs (1)
1039-1077: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate
normalize_markup_textinto one shared crate-internal helper. The function is implemented byte-for-byte identically in both files. Since the parser (ui.rs) normalizes annotation/comment text and the formatter (format.rs) re-normalizes it on output, the two copies must stay in lockstep or parse→format→reparse idempotency (asserted intests/annotations.rs) silently breaks. Extract once and reuse to make that invariant structural rather than convention.
crates/uhura-syntax/src/ui.rs#L1039-L1077: keep this as the single definition (or move it to a shared module) and expose itpub(crate).crates/uhura-syntax/src/format.rs#L1549-L1586: remove the duplicate and call the shared helper.🤖 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 `@crates/uhura-syntax/src/ui.rs` around lines 1039 - 1077, Deduplicate normalize_markup_text by making the definition in crates/uhura-syntax/src/ui.rs#L1039-L1077 pub(crate), or moving it to an appropriate shared module, while preserving its behavior. Remove the duplicate implementation in crates/uhura-syntax/src/format.rs#L1549-L1586 and update formatter call sites to use the shared helper.crates/uhura-check/src/provenance.rs (1)
1053-1095: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated "invisible node" logic vs.
ui_node_is_source_only.The inner merge loop re-derives which nodes are invisible (comments, whitespace-only text) instead of delegating to
ui_node_is_source_only, which the outer loop already uses for the same purpose (Line 1060-1063). If that helper's semantics change, this loop can silently diverge and coalesce a different set of nodes into the merged text span.♻️ Suggested consolidation
while let Some(candidate) = nodes.get(next) { match &candidate.kind { - ast::UiNodeKind::Comment(_) => next += 1, - ast::UiNodeKind::Text(value) - if value.raw.chars().all(char::is_whitespace) => - { - next += 1; - } ast::UiNodeKind::Text(_) => { - span = span.through(candidate.span); - next += 1; + if !ui_node_is_source_only(candidate) { + span = span.through(candidate.span); + } + next += 1; } + _ if ui_node_is_source_only(candidate) => next += 1, _ => break, } }🤖 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 `@crates/uhura-check/src/provenance.rs` around lines 1053 - 1095, Update the text-merging loop in the provenance-building method to use ui_node_is_source_only(candidate) for invisible-node detection, while preserving the existing behavior of merging non-whitespace text nodes and stopping at other semantic nodes. Remove the duplicated comment/whitespace classification so the outer traversal and inner merge loop share the same helper semantics.
🤖 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 `@crates/uhura-check/src/parts.rs`:
- Around line 1887-1896: Populate the unit-update map before constructing
MachinePlan, using the existing unit-update collection produced during machine
planning rather than BTreeMap::new(). Pass that populated map into
MachinePlan.unit_updates so reject_unit_update_cycles and downstream unit-update
inlining receive the collected updates.
In `@crates/uhura-check/src/provenance.rs`:
- Around line 200-220: Remove the eager lowering-name lookup loop that populates
lowering_names, and instead resolve a declaration’s lowered name inside
visit_declaration only when handling private UI declarations. Preserve direct
name.text usage for public declarations, while retaining the existing error when
a private declaration lacks a lowering-name binding.
---
Nitpick comments:
In `@crates/uhura-check/src/provenance.rs`:
- Around line 1053-1095: Update the text-merging loop in the provenance-building
method to use ui_node_is_source_only(candidate) for invisible-node detection,
while preserving the existing behavior of merging non-whitespace text nodes and
stopping at other semantic nodes. Remove the duplicated comment/whitespace
classification so the outer traversal and inner merge loop share the same helper
semantics.
In `@crates/uhura-syntax/src/ui.rs`:
- Around line 1039-1077: Deduplicate normalize_markup_text by making the
definition in crates/uhura-syntax/src/ui.rs#L1039-L1077 pub(crate), or moving it
to an appropriate shared module, while preserving its behavior. Remove the
duplicate implementation in crates/uhura-syntax/src/format.rs#L1549-L1586 and
update formatter call sites to use the shared helper.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0da37ca-f892-4603-8834-e9a43fcc3f12
📒 Files selected for processing (72)
.github/workflows/ci.ymlcrates/uhura-base/src/codes.rscrates/uhura-check/src/authoring.rscrates/uhura-check/src/checker.rscrates/uhura-check/src/compile.rscrates/uhura-check/src/evidence.rscrates/uhura-check/src/lib.rscrates/uhura-check/src/parts.rscrates/uhura-check/src/project_manifest.rscrates/uhura-check/src/provenance.rscrates/uhura-check/src/source.rscrates/uhura-check/src/source/references.rscrates/uhura-check/src/source/ui.rscrates/uhura-check/src/topology.rscrates/uhura-check/src/updates.rscrates/uhura-check/tests/application_bridges.rscrates/uhura-check/tests/comments.rscrates/uhura-check/tests/compile.rscrates/uhura-check/tests/evidence.rscrates/uhura-check/tests/instagram.rscrates/uhura-check/tests/modules.rscrates/uhura-check/tests/packages.rscrates/uhura-check/tests/parts.rscrates/uhura-check/tests/programs.rscrates/uhura-check/tests/project_lock.rscrates/uhura-check/tests/project_manifest.rscrates/uhura-check/tests/provenance.rscrates/uhura-check/tests/returns.rscrates/uhura-check/tests/standard.rscrates/uhura-check/tests/ui.rscrates/uhura-cli/src/cmd/fmt.rscrates/uhura-cli/src/cmd/project.rscrates/uhura-core/src/ir.rscrates/uhura-core/src/runtime.rscrates/uhura-host/src/lib.rscrates/uhura-host/src/source.rscrates/uhura-syntax/src/ast.rscrates/uhura-syntax/src/format.rscrates/uhura-syntax/src/lexer.rscrates/uhura-syntax/src/lib.rscrates/uhura-syntax/src/parser.rscrates/uhura-syntax/src/ui.rscrates/uhura-syntax/src/v04/mod.rscrates/uhura-syntax/tests/annotations.rscrates/uhura-syntax/tests/fixtures/feed-ui.uhuracrates/uhura-syntax/tests/format.rscrates/uhura-syntax/tests/instagram.rscrates/uhura-syntax/tests/lexer.rscrates/uhura-syntax/tests/parser.rscrates/uhura-syntax/tests/programs.rscrates/uhura-syntax/tests/ui.rscrates/uhura-tests/tests/a0.rscrates/uhura-tests/tests/diagnostic_goldens.rscrates/uhura-tests/tests/fixtures/diagnostics/name-resolution.jsoncrates/uhura-tests/tests/fixtures/diagnostics/name-resolution.uhuracrates/uhura-tests/tests/fixtures/diagnostics/parse-fix-declaration-typo.jsoncrates/uhura-tests/tests/fixtures/diagnostics/parse-fix-declaration-typo.uhuracrates/uhura-tests/tests/fixtures/diagnostics/parse-invalid-expression.jsoncrates/uhura-tests/tests/fixtures/diagnostics/parse-invalid-expression.uhuracrates/uhura-tests/tests/fixtures/diagnostics/type-mismatch.jsoncrates/uhura-tests/tests/fixtures/diagnostics/type-mismatch.uhuracrates/uhura-tests/tests/fixtures/diagnostics/ui-content.jsoncrates/uhura-tests/tests/fixtures/diagnostics/ui-content.uhuracrates/uhura-tests/tests/instagram.rscrates/uhura-wasm/tests/dispatch_scaling.rsdocs/implementation/README.mddocs/rfcs/0003-source-comments-docs-and-annotations.mddocs/spec/drafts/0.4/source.mdexamples/instagram/client/styles/theme.cssexamples/instagram/client/ui.uhuraweb/src/editor/editor-updates.tsweb/src/editor/tests/editor-updates.test.ts
💤 Files with no reviewable changes (1)
- crates/uhura-syntax/src/v04/mod.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/uhura-syntax/src/ui.rs (1)
1039-1077: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate
normalize_markup_textinto one shared crate-internal helper. The function is implemented byte-for-byte identically in both files. Since the parser (ui.rs) normalizes annotation/comment text and the formatter (format.rs) re-normalizes it on output, the two copies must stay in lockstep or parse→format→reparse idempotency (asserted intests/annotations.rs) silently breaks. Extract once and reuse to make that invariant structural rather than convention.
crates/uhura-syntax/src/ui.rs#L1039-L1077: keep this as the single definition (or move it to a shared module) and expose itpub(crate).crates/uhura-syntax/src/format.rs#L1549-L1586: remove the duplicate and call the shared helper.🤖 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 `@crates/uhura-syntax/src/ui.rs` around lines 1039 - 1077, Deduplicate normalize_markup_text by making the definition in crates/uhura-syntax/src/ui.rs#L1039-L1077 pub(crate), or moving it to an appropriate shared module, while preserving its behavior. Remove the duplicate implementation in crates/uhura-syntax/src/format.rs#L1549-L1586 and update formatter call sites to use the shared helper.crates/uhura-check/src/provenance.rs (1)
1053-1095: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated "invisible node" logic vs.
ui_node_is_source_only.The inner merge loop re-derives which nodes are invisible (comments, whitespace-only text) instead of delegating to
ui_node_is_source_only, which the outer loop already uses for the same purpose (Line 1060-1063). If that helper's semantics change, this loop can silently diverge and coalesce a different set of nodes into the merged text span.♻️ Suggested consolidation
while let Some(candidate) = nodes.get(next) { match &candidate.kind { - ast::UiNodeKind::Comment(_) => next += 1, - ast::UiNodeKind::Text(value) - if value.raw.chars().all(char::is_whitespace) => - { - next += 1; - } ast::UiNodeKind::Text(_) => { - span = span.through(candidate.span); - next += 1; + if !ui_node_is_source_only(candidate) { + span = span.through(candidate.span); + } + next += 1; } + _ if ui_node_is_source_only(candidate) => next += 1, _ => break, } }🤖 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 `@crates/uhura-check/src/provenance.rs` around lines 1053 - 1095, Update the text-merging loop in the provenance-building method to use ui_node_is_source_only(candidate) for invisible-node detection, while preserving the existing behavior of merging non-whitespace text nodes and stopping at other semantic nodes. Remove the duplicated comment/whitespace classification so the outer traversal and inner merge loop share the same helper semantics.
🤖 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 `@crates/uhura-check/src/parts.rs`:
- Around line 1887-1896: Populate the unit-update map before constructing
MachinePlan, using the existing unit-update collection produced during machine
planning rather than BTreeMap::new(). Pass that populated map into
MachinePlan.unit_updates so reject_unit_update_cycles and downstream unit-update
inlining receive the collected updates.
In `@crates/uhura-check/src/provenance.rs`:
- Around line 200-220: Remove the eager lowering-name lookup loop that populates
lowering_names, and instead resolve a declaration’s lowered name inside
visit_declaration only when handling private UI declarations. Preserve direct
name.text usage for public declarations, while retaining the existing error when
a private declaration lacks a lowering-name binding.
---
Nitpick comments:
In `@crates/uhura-check/src/provenance.rs`:
- Around line 1053-1095: Update the text-merging loop in the provenance-building
method to use ui_node_is_source_only(candidate) for invisible-node detection,
while preserving the existing behavior of merging non-whitespace text nodes and
stopping at other semantic nodes. Remove the duplicated comment/whitespace
classification so the outer traversal and inner merge loop share the same helper
semantics.
In `@crates/uhura-syntax/src/ui.rs`:
- Around line 1039-1077: Deduplicate normalize_markup_text by making the
definition in crates/uhura-syntax/src/ui.rs#L1039-L1077 pub(crate), or moving it
to an appropriate shared module, while preserving its behavior. Remove the
duplicate implementation in crates/uhura-syntax/src/format.rs#L1549-L1586 and
update formatter call sites to use the shared helper.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0da37ca-f892-4603-8834-e9a43fcc3f12
📒 Files selected for processing (72)
.github/workflows/ci.ymlcrates/uhura-base/src/codes.rscrates/uhura-check/src/authoring.rscrates/uhura-check/src/checker.rscrates/uhura-check/src/compile.rscrates/uhura-check/src/evidence.rscrates/uhura-check/src/lib.rscrates/uhura-check/src/parts.rscrates/uhura-check/src/project_manifest.rscrates/uhura-check/src/provenance.rscrates/uhura-check/src/source.rscrates/uhura-check/src/source/references.rscrates/uhura-check/src/source/ui.rscrates/uhura-check/src/topology.rscrates/uhura-check/src/updates.rscrates/uhura-check/tests/application_bridges.rscrates/uhura-check/tests/comments.rscrates/uhura-check/tests/compile.rscrates/uhura-check/tests/evidence.rscrates/uhura-check/tests/instagram.rscrates/uhura-check/tests/modules.rscrates/uhura-check/tests/packages.rscrates/uhura-check/tests/parts.rscrates/uhura-check/tests/programs.rscrates/uhura-check/tests/project_lock.rscrates/uhura-check/tests/project_manifest.rscrates/uhura-check/tests/provenance.rscrates/uhura-check/tests/returns.rscrates/uhura-check/tests/standard.rscrates/uhura-check/tests/ui.rscrates/uhura-cli/src/cmd/fmt.rscrates/uhura-cli/src/cmd/project.rscrates/uhura-core/src/ir.rscrates/uhura-core/src/runtime.rscrates/uhura-host/src/lib.rscrates/uhura-host/src/source.rscrates/uhura-syntax/src/ast.rscrates/uhura-syntax/src/format.rscrates/uhura-syntax/src/lexer.rscrates/uhura-syntax/src/lib.rscrates/uhura-syntax/src/parser.rscrates/uhura-syntax/src/ui.rscrates/uhura-syntax/src/v04/mod.rscrates/uhura-syntax/tests/annotations.rscrates/uhura-syntax/tests/fixtures/feed-ui.uhuracrates/uhura-syntax/tests/format.rscrates/uhura-syntax/tests/instagram.rscrates/uhura-syntax/tests/lexer.rscrates/uhura-syntax/tests/parser.rscrates/uhura-syntax/tests/programs.rscrates/uhura-syntax/tests/ui.rscrates/uhura-tests/tests/a0.rscrates/uhura-tests/tests/diagnostic_goldens.rscrates/uhura-tests/tests/fixtures/diagnostics/name-resolution.jsoncrates/uhura-tests/tests/fixtures/diagnostics/name-resolution.uhuracrates/uhura-tests/tests/fixtures/diagnostics/parse-fix-declaration-typo.jsoncrates/uhura-tests/tests/fixtures/diagnostics/parse-fix-declaration-typo.uhuracrates/uhura-tests/tests/fixtures/diagnostics/parse-invalid-expression.jsoncrates/uhura-tests/tests/fixtures/diagnostics/parse-invalid-expression.uhuracrates/uhura-tests/tests/fixtures/diagnostics/type-mismatch.jsoncrates/uhura-tests/tests/fixtures/diagnostics/type-mismatch.uhuracrates/uhura-tests/tests/fixtures/diagnostics/ui-content.jsoncrates/uhura-tests/tests/fixtures/diagnostics/ui-content.uhuracrates/uhura-tests/tests/instagram.rscrates/uhura-wasm/tests/dispatch_scaling.rsdocs/implementation/README.mddocs/rfcs/0003-source-comments-docs-and-annotations.mddocs/spec/drafts/0.4/source.mdexamples/instagram/client/styles/theme.cssexamples/instagram/client/ui.uhuraweb/src/editor/editor-updates.tsweb/src/editor/tests/editor-updates.test.ts
💤 Files with no reviewable changes (1)
- crates/uhura-syntax/src/v04/mod.rs
🛑 Comments failed to post (2)
crates/uhura-check/src/parts.rs (1)
1887-1896: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Find any writes to MachinePlan.unit_updates / UnitUpdate construction across the crate. rg -nP 'unit_updates\s*[:=.]|UnitUpdate\s*\{' crates/uhura-check/srcRepository: gridaco/uhura
Length of output: 903
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate all reads/writes of the MachinePlan.unit_updates field and inspect the # surrounding code paths that should populate it. rg -n --context 3 '\.unit_updates\b|unit_updates\s*:' crates/uhura-check/src/parts.rsRepository: gridaco/uhura
Length of output: 2611
🏁 Script executed:
#!/bin/bash set -euo pipefail # Search the whole repository for any writes/population of unit_updates. rg -n --context 2 '\bunit_updates\b' . \ | rg -n 'insert|extend|push|clear|remove|entry|BTreeMap::new|UnitUpdate'Repository: gridaco/uhura
Length of output: 1058
Populate
unit_updatesbefore constructingMachinePlan
MachinePlan.unit_updatesis always empty here, and there’s no write to it anywhere incrates/uhura-check/src/parts.rs, soreject_unit_update_cyclesand the unit-update inlining path never run.🤖 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 `@crates/uhura-check/src/parts.rs` around lines 1887 - 1896, Populate the unit-update map before constructing MachinePlan, using the existing unit-update collection produced during machine planning rather than BTreeMap::new(). Pass that populated map into MachinePlan.unit_updates so reject_unit_update_cycles and downstream unit-update inlining receive the collected updates.crates/uhura-check/src/provenance.rs (1)
200-220: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Locate the definition and contract of lowered_declaration_name / lowered_declaration_name_mut rg -n -B2 -A15 'fn lowered_declaration_name\b' crates/uhura-check/src rg -n -B2 -A10 'fn lowered_declaration_name_mut\b' crates/uhura-check/srcRepository: gridaco/uhura
Length of output: 2408
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the lowering/metadata flow around bindings and declaration consumption. sed -n '1,140p' crates/uhura-check/src/source.rs printf '\n----\n' sed -n '300,380p' crates/uhura-check/src/provenance.rs printf '\n----\n' rg -n -A4 -B4 'bindings|lowered_declaration_name\(' crates/uhura-check/srcRepository: gridaco/uhura
Length of output: 50370
Defer the lowering-name lookup to the private UI path The eager loop in
crates/uhura-check/src/provenance.rs:200-220hard-fails on any declaration without a binding entry, butvisit_declarationonly readslowering_namesforVisibility::PrivateUI declarations; public declarations usename.textdirectly, so valid public items can fail provenance building unnecessarily.🤖 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 `@crates/uhura-check/src/provenance.rs` around lines 200 - 220, Remove the eager lowering-name lookup loop that populates lowering_names, and instead resolve a declaration’s lowered name inside visit_declaration only when handling private UI declarations. Preserve direct name.text usage for public declarations, while retaining the existing error when a private declaration lacks a lowering-name binding.
1 participant
Summary
<!-- @annotation ... -->as checked source metadata and attach them deterministically to supported UI targets.v04generation naming from syntax, checking, compilation, provenance, consumers, tests, fixtures, and public Rust APIs.Why
Source annotations are authoring information: the Editor needs their exact source attachment, but they must not enter machine behavior, program identity, or rendered document semantics. The checked frontend now owns that attachment and exposes a deterministic authoring sidecar.
Separately, Uhura has only one admitted frontend. Internal
v04modules and helper names implied a compatibility layer that does not exist and made the implementation topology harder to understand. The frontend now uses canonical, unversioned internal names.Impact
Validation
cargo fmt --all -- --checkcargo test --locked --workspace --all-targetscargo clippy --locked --workspace --all-targets -- -D warningspnpm checkcargo run --locked -q -p uhura-cli -- check examples/instagram/client --deny-warningsv04,v0.4, and0_4namesgit diff --check