refactor(ergo-compiler): split four oversized backend files into submodule directories - #202
Merged
Merged
Conversation
…odule directories
typer/assign.rs (3987 lines), emit.rs (3632), cse.rs (2952), and tree.rs (2434)
had each grown into a single file spanning multiple independently-testable
phases. Splits each along its own natural phase/subsystem boundaries into a
directory of submodules (assign/, emit/, cse/, tree/), keeping cross-cutting
pass-ordering rationale (graph_build) and Interner/Scope-style impl blocks
correctly wired across files.
Pure move: no logic changes. Each split was extracted incrementally (baseline
test run before starting, cargo test after every file extraction), and the
whole batch independently re-verified by four separate Opus red-team passes
that diffed every function body against the pre-refactor original and
re-ran the full gate themselves. Two things worth calling out since they're
not obvious from the diff:
- cse.rs: Interner/SymId were genuinely part of the crate's external public
API (pub mod cse; plus plain `pub struct`/`pub fn`) — caught via
`clippy --all-targets` reporting dead_code once a first-pass `pub(crate)
use` re-export silently downgraded their external reachability; fixed to
`pub use`.
- tree.rs: `crate::fold` imports `crate::tree::{in_fold_range, FoldWidth}`
directly — confirmed this cross-module path still resolves after
in_fold_range/FoldWidth moved into tree/cast_fold.rs.
Full gate green throughout: `cargo fmt --all -- --check`,
`cargo clippy --workspace --all-targets --all-features -- -D warnings`,
`cargo test --workspace` (294 test-result blocks, 0 failed, matching the
803-test ergo-compiler lib baseline captured before touching anything).
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughThe pull request adds typed-expression assignment, opcode emission, tree validation and assembly, and a scoped common-subexpression elimination pipeline with symbol interning, hoisting gates, materialization, and extensive parity tests. ChangesTyped expression pipeline
Emission and tree pipeline
Common-subexpression elimination
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TypedTyper
participant Emitter
participant TreePipeline
participant CSEInterner
participant Materializer
TypedTyper->>Emitter: assign typed expressions
Emitter->>TreePipeline: emit opcode IR
TreePipeline->>CSEInterner: intern transformed expression
CSEInterner->>Materializer: materialize root symbol
Materializer->>TreePipeline: return rebuilt IR
TreePipeline->>TreePipeline: fold, validate, segregate, and assemble tree
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@ergo-compiler/src/cse/key.rs`:
- Around line 9-10: Update the intra-doc links in key.rs, including the links
near the symbol identity documentation and the other referenced locations, to
qualify Interner as super::Interner. Do not change the surrounding documentation
text or code.
In `@ergo-compiler/src/cse/mod.rs`:
- Around line 194-202: Update the documentation references in
ergo-compiler/src/cse/mod.rs lines 194-202 to use the relocated emission module
and current symbol instead of emit.rs and its obsolete line reference; also
update the emit_block reference in ergo-compiler/src/cse/materialize.rs lines
432-434 to point to its new module. No code behavior changes are needed.
- Around line 1-9: Update the module-level documentation in the CSE module to
describe the complete pass implemented by gate, materialize, and cse(),
including interning, scope handling, usage counting, and ValDef materialization.
Remove outdated statements that limit the module to a substrate or claim those
stages are not implemented, while preserving any accurate scope and non-Scalan
distinctions.
In `@ergo-compiler/src/emit/dispatch.rs`:
- Around line 229-248: In the SigmaProp-element branch of T::XorOf, change the
emitted error classification from UnsupportedNode to GraphBuildingReject while
preserving the existing detection and explanatory message. Keep the surrounding
XorOf handling unchanged so callers retain the reference compiler’s
AssertionError classification.
In `@ergo-compiler/src/typer/assign/apply.rs`:
- Around line 61-66: Update the global-method branch in the application typer,
using global_method and process_global_method, to validate the typed arguments
against the resolved SGlobal method signature before lowering. Enforce both
arity and argument-type checks, return the standard typing error for invalid
calls, and only construct the lowered global method expression after validation
succeeds.
In `@ergo-compiler/src/typer/assign/harness.rs`:
- Around line 32-35: Update the numeric_diff bypass in the assignment harness to
require the operation’s t_arg to represent a polymorphic numeric operation
before allowing mixed numeric types. Ensure directly constructed Xor(Int, Long)
still undergoes the required Coll[Byte] validation, while preserving widening
for valid polymorphic numeric calls.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a0db6243-acc8-4463-89cd-1498d018f44c
📒 Files selected for processing (29)
ergo-compiler/src/cse.rsergo-compiler/src/cse/codec.rsergo-compiler/src/cse/gate.rsergo-compiler/src/cse/intern.rsergo-compiler/src/cse/interner.rsergo-compiler/src/cse/key.rsergo-compiler/src/cse/materialize.rsergo-compiler/src/cse/mod.rsergo-compiler/src/emit/dispatch.rsergo-compiler/src/emit/method_call.rsergo-compiler/src/emit/mod.rsergo-compiler/src/emit/scope.rsergo-compiler/src/emit/select.rsergo-compiler/src/emit/types.rsergo-compiler/src/tree/assemble.rsergo-compiler/src/tree/cast_fold.rsergo-compiler/src/tree/lambda_gate.rsergo-compiler/src/tree/mod.rsergo-compiler/src/tree/v0_gate.rsergo-compiler/src/tree/walk.rsergo-compiler/src/typed.rsergo-compiler/src/typer/assign.rsergo-compiler/src/typer/assign/apply.rsergo-compiler/src/typer/assign/arith_bitop.rsergo-compiler/src/typer/assign/harness.rsergo-compiler/src/typer/assign/lower_method.rsergo-compiler/src/typer/assign/method_call_like.rsergo-compiler/src/typer/assign/mod.rsergo-compiler/src/typer/assign/simple_arms.rs
Comment on lines
+229
to
+248
| T::XorOf { input, .. } => { | ||
| // Verdict parity with the FULL Scala compiler (Task-10 gate): | ||
| // the reference typer accepts `xorOf(Coll(sigmaProp(..)))` as | ||
| // `XorOf(ConcreteCollection[SigmaProp])` — the Bool↔SigmaProp | ||
| // unifier admits the elements WITHOUT the per-element | ||
| // `SigmaPropIsProven` coercion that `allOf`/`anyOf` get | ||
| // (golden_seed §14) — but GraphBuilding then force-casts the | ||
| // input to `Coll[Boolean]` (`asRep[Coll[Boolean]]` / | ||
| // `sigmaDslBuilder.xorOf`, GraphBuilding.scala:855-862) and | ||
| // dies with an `AssertionError`, so `compiler.compile` REJECTS | ||
| // every such source (oracle: `cc xorOf(Coll(sigmaProp(true)))` | ||
| // → `REJECT 0:0 AssertionError`, compile_seed.json). Mirror | ||
| // the verdict: a SigmaProp-element XorOf never reaches the | ||
| // wire. (0xFF's wire/eval semantics also require booleans.) | ||
| if matches!(node_tpe(input), SType::SColl(el) if **el == SType::SSigmaProp) { | ||
| return Err(EmitError::UnsupportedNode( | ||
| "XorOf over Coll[SigmaProp] (Scala GraphBuilding rejects: \ | ||
| xorOf input must be Coll[Boolean], GraphBuilding.scala:855-862)" | ||
| .into(), | ||
| )); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the reference AssertionError classification.
Line 243 detects a user-reachable GraphBuilding rejection, but returns UnsupportedNode. Return GraphBuildingReject so callers do not mistake reference-compiler behavior for an unsupported emitter feature.
Proposed fix
- return Err(EmitError::UnsupportedNode(
- "XorOf over Coll[SigmaProp] (Scala GraphBuilding rejects: \
- xorOf input must be Coll[Boolean], GraphBuilding.scala:855-862)"
- .into(),
- ));
+ return Err(EmitError::GraphBuildingReject {
+ class: "AssertionError",
+ what: "XorOf input must be Coll[Boolean]; Scala GraphBuilding \
+ rejects Coll[SigmaProp]"
+ .into(),
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| T::XorOf { input, .. } => { | |
| // Verdict parity with the FULL Scala compiler (Task-10 gate): | |
| // the reference typer accepts `xorOf(Coll(sigmaProp(..)))` as | |
| // `XorOf(ConcreteCollection[SigmaProp])` — the Bool↔SigmaProp | |
| // unifier admits the elements WITHOUT the per-element | |
| // `SigmaPropIsProven` coercion that `allOf`/`anyOf` get | |
| // (golden_seed §14) — but GraphBuilding then force-casts the | |
| // input to `Coll[Boolean]` (`asRep[Coll[Boolean]]` / | |
| // `sigmaDslBuilder.xorOf`, GraphBuilding.scala:855-862) and | |
| // dies with an `AssertionError`, so `compiler.compile` REJECTS | |
| // every such source (oracle: `cc xorOf(Coll(sigmaProp(true)))` | |
| // → `REJECT 0:0 AssertionError`, compile_seed.json). Mirror | |
| // the verdict: a SigmaProp-element XorOf never reaches the | |
| // wire. (0xFF's wire/eval semantics also require booleans.) | |
| if matches!(node_tpe(input), SType::SColl(el) if **el == SType::SSigmaProp) { | |
| return Err(EmitError::UnsupportedNode( | |
| "XorOf over Coll[SigmaProp] (Scala GraphBuilding rejects: \ | |
| xorOf input must be Coll[Boolean], GraphBuilding.scala:855-862)" | |
| .into(), | |
| )); | |
| T::XorOf { input, .. } => { | |
| // Verdict parity with the FULL Scala compiler (Task-10 gate): | |
| // the reference typer accepts `xorOf(Coll(sigmaProp(..)))` as | |
| // `XorOf(ConcreteCollection[SigmaProp])` — the Bool↔SigmaProp | |
| // unifier admits the elements WITHOUT the per-element | |
| // `SigmaPropIsProven` coercion that `allOf`/`anyOf` get | |
| // (golden_seed §14) — but GraphBuilding then force-casts the | |
| // input to `Coll[Boolean]` (`asRep<Coll[Boolean]]` / | |
| // `sigmaDslBuilder.xorOf`, GraphBuilding.scala:855-862) and | |
| // dies with an `AssertionError`, so `compiler.compile` REJECTS | |
| // every such source (oracle: `cc xorOf(Coll(sigmaProp(true)))` | |
| // → `REJECT 0:0 AssertionError`, compile_seed.json). Mirror | |
| // the verdict: a SigmaProp-element XorOf never reaches the | |
| // wire. (0xFF's wire/eval semantics also require booleans.) | |
| if matches!(node_tpe(input), SType::SColl(el) if **el == SType::SSigmaProp) { | |
| return Err(EmitError::GraphBuildingReject { | |
| class: "AssertionError", | |
| what: "XorOf input must be Coll[Boolean]; Scala GraphBuilding \ | |
| rejects Coll[SigmaProp]" | |
| .into(), | |
| }); |
🤖 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 `@ergo-compiler/src/emit/dispatch.rs` around lines 229 - 248, In the
SigmaProp-element branch of T::XorOf, change the emitted error classification
from UnsupportedNode to GraphBuildingReject while preserving the existing
detection and explanatory message. Keep the surrounding XorOf handling unchanged
so callers retain the reference compiler’s AssertionError classification.
Comment on lines
+61
to
+66
| // §1.9 — Apply(Ident, args) if SGlobalMethods.hasMethod(ident.name). | ||
| if let TypedExpr::Ident { name, .. } = &func { | ||
| if let Some(method) = global_method(name, ctx.tree_version) { | ||
| let new_args = type_all(env, args, ctx)?; | ||
| return process_global_method(&method, new_args); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate bare SGlobal arguments before lowering.
This path skips the signature checks used by every other application route. For example, xor(1, 2) becomes a Coll[Byte]-typed Xor with integer children, while wrong-arity calls can become malformed MethodCalls.
Proposed fix
if let TypedExpr::Ident { name, .. } = &func {
if let Some(method) = global_method(name, ctx.tree_version) {
let new_args = type_all(env, args, ctx)?;
+ let expected = method.stype.dom_tail();
+ let actual: Vec<SType> =
+ new_args.iter().map(|arg| node_tpe(arg).clone()).collect();
+ if unify_type_lists(expected, &actual).is_none() {
+ return Err(TyperError::typer(format!(
+ "For global method {name} expected args: {expected:?}; actual: {actual:?}"
+ )));
+ }
return process_global_method(&method, new_args);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // §1.9 — Apply(Ident, args) if SGlobalMethods.hasMethod(ident.name). | |
| if let TypedExpr::Ident { name, .. } = &func { | |
| if let Some(method) = global_method(name, ctx.tree_version) { | |
| let new_args = type_all(env, args, ctx)?; | |
| return process_global_method(&method, new_args); | |
| } | |
| // §1.9 — Apply(Ident, args) if SGlobalMethods.hasMethod(ident.name). | |
| if let TypedExpr::Ident { name, .. } = &func { | |
| if let Some(method) = global_method(name, ctx.tree_version) { | |
| let new_args = type_all(env, args, ctx)?; | |
| let expected = method.stype.dom_tail(); | |
| let actual: Vec<SType> = | |
| new_args.iter().map(|arg| node_tpe(arg).clone()).collect(); | |
| if unify_type_lists(expected, &actual).is_none() { | |
| return Err(TyperError::typer(format!( | |
| "For global method {name} expected args: {expected:?}; actual: {actual:?}" | |
| ))); | |
| } | |
| return process_global_method(&method, new_args); | |
| } |
🤖 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 `@ergo-compiler/src/typer/assign/apply.rs` around lines 61 - 66, Update the
global-method branch in the application typer, using global_method and
process_global_method, to validate the typed arguments against the resolved
SGlobal method signature before lowering. Enforce both arity and argument-type
checks, return the standard typing error for invalid calls, and only construct
the lowered global method expression after validation succeeds.
Comment on lines
+32
to
+35
| // (numeric, numeric) with t1 != t2 -> allowed (the builder inserts Upcast); | ||
| // else the unify enforces the concrete/consistent arg types. | ||
| let numeric_diff = is_numeric(<) && is_numeric(&rt) && lt != rt; | ||
| if !numeric_diff { |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restrict mixed-numeric widening to polymorphic numeric operations.
The current bypass ignores t_arg. Consequently, a directly constructed Xor(Int, Long) skips the required Coll[Byte] check and is accepted as byte-array XOR.
Proposed fix
- let numeric_diff = is_numeric(<) && is_numeric(&rt) && lt != rt;
+ let numeric_diff = matches!(&t_arg, SType::STypeVar(_))
+ && is_numeric(<)
+ && is_numeric(&rt)
+ && lt != rt;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // (numeric, numeric) with t1 != t2 -> allowed (the builder inserts Upcast); | |
| // else the unify enforces the concrete/consistent arg types. | |
| let numeric_diff = is_numeric(<) && is_numeric(&rt) && lt != rt; | |
| if !numeric_diff { | |
| // (numeric, numeric) with t1 != t2 -> allowed (the builder inserts Upcast); | |
| // else the unify enforces the concrete/consistent arg types. | |
| let numeric_diff = matches!(&t_arg, SType::STypeVar(_)) | |
| && is_numeric(<) | |
| && is_numeric(&rt) | |
| && lt != rt; | |
| if !numeric_diff { |
🤖 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 `@ergo-compiler/src/typer/assign/harness.rs` around lines 32 - 35, Update the
numeric_diff bypass in the assignment harness to require the operation’s t_arg
to represent a polymorphic numeric operation before allowing mixed numeric
types. Ensure directly constructed Xor(Int, Long) still undergoes the required
Coll[Byte] validation, while preserving widening for valid polymorphic numeric
calls.
…iles Reference-implementation-standard documentation pass over typer/assign/*, emit/*, cse/*, tree/*, and typed.rs (the files touched by the split PR). Comment-only changes: no logic, behavior, formatting-beyond-comments, or naming changed. Removed development-history narration, AI-agent/task-tracker labels, bare capture dates, and a stray upstream issue-tracker reference. Kept every Scala-citation, oracle test-vector, and deviation-ledger comment intact — this crate's correctness rests on byte-parity with a Scala reference implementation, so those are protocol documentation, not history. Corrected several comments the split had left stale or that were simply wrong about current behavior: - cse/mod.rs's module doc claimed CSE "does NOT emit ValDefs... NOT wired into compile() yet" and two corpus tests claimed their oracle vectors "stay SET-listed until Task 4 wires CSE live" — both false; CSE has been fully wired and live since this session's cse.rs split. - Stale cross-file citations left over from the four splits (bare `emit.rs:NNN`/`tree.rs:NNN`/`assign.rs:NNN` line references that moved) repointed to the correct module. - Four broken rustdoc intra-doc links in cse/key.rs (`[Interner::...]` referencing a sibling module's type not in scope) qualified as `[super::Interner::...]`, confirmed resolving via `cargo doc`. Three logic findings from CodeRabbit review (emit/dispatch.rs's XorOf error classification, typer/assign/apply.rs's global-method arg validation, typer/assign/harness.rs's numeric_diff bypass) were confirmed pre-existing via git history and left untouched — they're behavior questions outside this PR's zero-behavior-change scope, not doc/move questions. Full gate green: cargo fmt --check, clippy --workspace --all-targets --all-features -D warnings, cargo test --workspace (294 test-result blocks, 0 failed), cargo test --doc (3/3 passed).
…from comments
Second documentation pass over the same files (typer/assign/*, emit/*,
cse/*, tree/*): comment-only, no logic/behavior/formatting/naming changes.
Removed references to repository-local design/planning artefacts that
aren't part of the crate's public contract: dev-docs/*.md spec citations,
the "spike §N" design-doc pointers (the densest pattern, concentrated in
cse/*), m5-sched-*.md/m5-root-schedule-order.md analysis docs,
recon-*.md, adversarial-findings-*.md, task-1-report.md, and the bare
finding codes (F1/F2/F6/NF-1/NF-2) that existed only to anchor those
docs. Every removal restates the invariant or rationale directly in the
comment instead of pointing at the internal doc.
Kept everywhere: Scala reference-implementation source citations,
oracle-captured byte vectors, the checked-in golden_seed.txt/
compile_seed.json test-vector fixtures (canonical oracle data, not
planning notes), and the D-C*/D-T* deviation-ledger codes (these
cross-reference lib.rs's own in-source ledger of real, permanent,
intentional deviations from the Scala reference -- a specification of
externally observable behavior, not a development artefact).
Incidentally rewrote two comments in tree/mod.rs that narrated bugs
already fixed ("It USED to gate on X... premise was FALSE") to state
only the current, verified behavior.
Full gate green: cargo fmt --check, clippy --workspace --all-targets
--all-features -D warnings, cargo test --workspace (294 test-result
blocks, 0 failed), cargo test --doc (3/3 passed).
arkadianet
marked this pull request as ready for review
July 16, 2026 10:17
21 tasks
arkadianet
added a commit
that referenced
this pull request
Jul 16, 2026
…203) * docs(ergo-compiler): reference-implementation documentation pass (remaining files) Elevates the last ~30 uncovered ergo-compiler files (parse/*, typer core, AST/type/token core, and the transform layer + lib.rs) to the same documentation standard applied in #202: strips internal planning-artifact references (dev-docs citations, milestone/task-tracker labels, dangling finding codes) while preserving all Scala source citations, oracle vectors, and the crate's deviation ledger. Doc/comment-only, no behavior changes. Also fixes one real inaccuracy found along the way: lib.rs's module doc claimed CSE was not yet wired into compile() -- it is (tree/mod.rs:217). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-crypto): strip internal-artifact citations from test comments Removes Task-N labels and a dangling internal-report/design-doc citation ("Task-4 report", "g25-pegmint-packaging §5.2.5") from group_element.rs and merkle/mod.rs, restating the same facts directly. Doc-only; all Scala/scrypto citations and oracle vectors untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-difftest): strip internal-artifact references from doc comments Removes bare bug-tracker numbers (#97, #108/#115, bug #6), a dangling "gitignored dev-docs Autolykos note" pointer, and redundant citations into interface-contracts.md/findings-and-triage.md whose content was already stated inline, restating each as a direct technical claim. Doc-only. Preserved: all Scala/JVM-oracle protocol documentation, and the extensive Bug #N cross-references in gen/mod.rs, gen/sigma_expr.rs, ergo_tree.rs, box_candidate.rs, transaction.rs, header.rs, constant.rs, asm.rs -- these map to the checked-in ergo-difftest/docs/known-bug-catalog.md rediscovery gate and are the crate's own live API contract (Feature::bug_id()), not development history. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-wallet): strip internal-artifact references from doc comments Removes a bare uncited bug label, "Task 38" tracker references, and roadmap-style "lands in following PRs"/"a later slice" phrasing from scan/mod.rs, scan/predicate.rs, address.rs, secret.rs, and storage.rs -- restated each as a direct statement of current scope. Doc-only. Preserved: all Fiat-Shamir/Schnorr/DHT protocol documentation, BIP32/BIP39/ EIP-3 citations, and the upstream Ergo issue #1627 legacy-derivation provenance baked into ExtendedSecretKeyLegacy/use_pre_1627. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-mining): strip internal design-doc citations from doc comments Removes dangling "v12 §N" / "design §N" internal design-plan citations from candidate.rs and handle.rs, and an uncited "audit-1" tracker label from error.rs -- restated each as a direct statement of the invariant. Doc-only. Preserved: all Scala consensus citations (CandidateGenerator.scala, EmissionRules.scala, ReemissionRules.scala), emission/coinbase/reward-script byte-layout documentation, and the "Component B" subsystem name where it functions as this crate's (and ergo-mempool's) stable cross-file name for the suspect-feed/targeted-recheck mechanism rather than a dangling doc pointer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-mempool): strip internal-artifact references from doc comments Removes this repo's own PR #139 references, a dangling "§7"/"Phase 2A guidance" design-doc citation, and two code-review-thread references ("Item 3 of the code-review fixes", "reviewer finding 1") from lib.rs and admission/tests.rs -- restated each as a direct technical statement. Doc-only. Preserved: all Scala mempool-parity citations (OrderedTxPool, MempoolAuditor, CleanupWorker), the "Component B" and numbered admission-step naming (both confirmed stable, cross-referenced internal pipeline structure, not dangling doc pointers), and mempool invariant #7's cross-reference to its real definition in admission.rs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-p2p): strip internal-artifact references, fix stale connection-limit doc Removes dangling internal labels (Sync-S3/Lever 1/plan §240 in sync.rs, Sync-S2 in delivery.rs, an AI-audit-session narrative and a "checklist contract" pointer in peer_manager/mod.rs) and roadmap phrasing in handshake.rs test docs, restating each as a direct technical statement. Doc-only. Also fixes a real inaccuracy: peer_manager/mod.rs's module doc still said "Max 80 total / 60 outbound" connections by default; the actual Default (limits.rs) is 384 total / 96 outbound / up to 256 inbound (decoupled). Updated to match. Preserved: all Scala P2P-protocol citations, wire-format byte-layout documentation, the out96/in256/cap384 connectivity-limit rationale, and the 2026-07-04 testnet-stall regression note in throttle.rs (genuine incident documentation, not development history). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-indexer): strip internal-artifact references from doc comments Removes a dangling "audit-2 M11" milestone label from handle.rs, and an AI-agent reference plus two citations to an uncommitted internal spec file (2026-05-01-storage-rent-eligibility.md, confirmed absent from the repo) from rollback.rs -- restated each as a direct technical statement. Doc-only. Preserved: all Scala indexer-parity citations, the testnet-431,366 and h=740,362 mainnet-incident notes, and the crate's own stable Phase 0/Phase 1 rebuild-state naming. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-sync): strip internal-artifact references from doc comments Removes AI-agent/review-tool references ("Codex supervisor plan", "codex review notes", "codex round-N guard"), internal task-tracker labels ("M5 final slice tracked in audit-todo"), and dangling internal increment/design-plan labels ("Sync-S0/S1/S3", "Plan §240") from executor/mod.rs, block_proc.rs, and coordinator/{mod,tests}.rs -- restated each as a direct technical statement. Doc-only. Preserved: all Scala sync-parity citations (ToDownloadProcessor.scala, ElementPartitioner.distribute), and the operational rationale behind the real merged "headers-synced stale-tip stall" and "caught-up-to-peers fallback" fixes, which document actual observed behavior rather than development history. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-validation): strip internal-artifact references, fix mojibake Removes dangling internal-plan/spec citations across block/header, voting, popow, and tx modules ("v12 §5 step N", "spec §8.1", "2026-04-28-voted- parameters-phase2.md" (confirmed absent from the repo), "R5 security gap / §11", "14.10", "§6.3", "T4 live differential", "codex P0-1", "§3.5") and an AI-review-tool tracker label, restating each as a direct technical statement. Doc-only. Also fixes a real encoding bug: voting/votes.rs and its oracle test carried double-encoded UTF-8 em-dashes ("—" instead of "—") -- restored throughout both files. The same corruption exists in ergo-sigma and will be fixed when that crate's pass runs. Preserved: every Scala consensus citation (ErgoStateContext.scala, NipopowAlgos.scala, Parameters.scala, RuleStatusSerializer.scala, etc.), mainnet-incident-derived rationale (blocks 290684/422179/1802240, h=1821696), EIP-27/storage-rent documentation, and all oracle-pinned test vectors -- this is the workspace's consensus-validation crate and none of its correctness specification was touched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-ser): strip internal-artifact references, fix broken citation Removes AI-review-tool tracker labels ("codex P1", "codex review of the MAX_EXPR_DEPTH=110 fix") from ergo_tree.rs and sigma_value.rs, and dangling citations to internal spec files confirmed absent from the repo (.superpowers/sdd/task-1-report.md, dev-docs/context-ext-count-signedness- recon.md, dev-docs/.../recon-segregation.md, "Phase 0 §11.5") from address.rs, input.rs, opcode/tests.rs, and popow_proof.rs. Also drops dangling "sub-phase 14.3"/"§14.3" cross-crate milestone labels from popow_header.rs, matching the same cleanup already done in ergo-validation's popow module. Doc-only. Also fixes a real broken citation: extension.rs's proptest doc comment named two test functions that don't exist in the file (extension_too_many_fields_returns_invalid_data / extension_value_too_long_returns_invalid_data); corrected to the actual names (extension_field_count_above_u16_returns_invalid_data / extension_field_value_above_255_returns_invalid_data). Preserved: every Scala/sigma-state wire-format citation, opcode-by-opcode byte-layout documentation, oracle-derived test vectors, the KMZ17 interlinks-sizing rationale, and the STypeVar/JVM-UTF8 and MAX_TYPE_DEPTH divergence notes -- this crate's entire purpose is byte-exact parity with the Scala reference and none of that specification was touched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-sigma): fix mojibake, strip internal-artifact references Fixes 42 instances of double-encoded UTF-8 mojibake ("—" -> "—") in evaluator/opcodes/method_call.rs, matching the same corruption already fixed in ergo-validation. Removes AI-review-tool/PR-number dev-history references from evaluator/tests.rs (codex P1, "per CodeRabbit on PR #38", "Reviewer finding:", a bare commit hash), evaluator/opcodes/method_call.rs ("PR #13/#14 oracle vectors"), and evaluator/opcodes/binding.rs ("CodeRabbit PR #161 finding") -- restated each as a direct technical statement. Doc-only. Preserved: every Scala/sigma-state citation across the evaluator (opcode dispatch, cost accounting, method_call semantics, Schnorr/DHT proof verification, AVL+ operations, verify.rs's top-level reduction path), oracle-pinned test vectors, and the GHSA-hfj8-hjph-7r78 security-advisory citation. Left one "TODO v6.0: implement" comment untouched in evaluator/opcodes/errors.rs -- it's a verbatim quote of the Scala reference source's own class comment (trees.scala:77), not this repo's dev history. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-state): strip internal-artifact references from doc comments Removes AI-review-tool attributions (Codex flagged/identified, "codex risk flag in 2h plan"), dangling internal task/milestone labels (Task 1.6/1.7, Phase 0/1a/1b/1c/2/2a/2b/3/3a/3b/4/5, sub-phase 14.5/14.10, audit-2 M5, Task 38), and citations to internal spec/incident docs confirmed absent from the repo (spec §7.1/§7.4, "2026-05-02-voted-params-first-epoch- boundary", "dev-docs/incident-2026-06-11-adproofs/") across the store, digest, avl, wallet, and persist modules -- restated each as a direct technical statement. Doc-only. Preserved: every Scala consensus citation, the crate's own stable Mode 2/3/5/6 operational-mode naming and per-function "Phase 1/2/3" step labels (these describe a single function's own algorithm steps, not a development roadmap -- same pattern as ergo-sync's kept "Step 2.5"), the mainnet incident at height 1,805,523 (technical substance kept, only the dangling doc pointer dropped), and all AVL+/digest-mode byte-layout documentation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-api): strip internal-artifact references, fix 3 stale docs Removes a systemic pattern found across nearly every v1/* file: citations to dev-docs/v1-api-design.md and its section numbers (§N.N), work-breakdown codes (G-N/O-N used as dangling pointers), "locked decision" labels, and a sibling dev-docs/v1-design-fragments/*.md fragment doc -- all confirmed absent from the repo. Also strips compat/blockchain module citations to a nonexistent "spec inventory"/section-12 label, and AI-review-tool references (CodeRabbit #170, "codex", "see the PR report"). Doc-only. Fixes 4 real staleness bugs found along the way: - v1/mod.rs: said v1 "isn't mounted on a route yet" -- it is (server.rs). - v1/auth.rs: said tiers/boot-warn aren't wired per-group yet -- they are. - v1/mempool_depth.rs: called stats/mempool-depth "future" -- it's live. - v1/realtime/bus.rs: called webhooks "a future PR" -- webhooks is built and is itself a live RealtimeBus subscriber. Preserved: every Scala/REST-compat citation, the compat/ module's byte-for-byte quirk-compatibility documentation, T0/T1/T2 tier naming and G-N/O-N primitive-numbering (real, pervasively cross-referenced internal names, not dangling doc pointers), and all storage-rent/wallet/mempool correctness rationale. Two residuals flagged but intentionally NOT touched (would be a behavior change, out of scope for a docs-only pass): three JSON response `detail`/ `note` string literals in operator/node.rs, accounts/mod.rs, and script/handlers.rs leak the same internal jargon into live API responses; and decode/registry.rs's `rent` protocol entry has `reference: "dev-docs/demurrage"`, a nonexistent path returned live via GET /api/v1/protocols. Both are real product bugs worth a follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * docs(ergo-node): strip internal-artifact references, fix 2 stale docs Removes AI-review-tool attributions (Codex plan finding/follow-up/audit citations, CodeRabbit PR #152), dangling internal task/milestone labels (Phase 0/1a/1b/2a/2b/2f-1/2f-3/2j/3a/3b/4/4a/4b/4d, sub-phase 14.6/14.10, Part 2 spec/§N, "M5 final slice in audit-todo", OBS-1/P2 work-item codes, bare commit hashes, "spec §7.3/§7.4"), and citations to internal design/spec docs confirmed absent from the repo (design §2/§5/§6/§6.2, "operator workload §D", "spec §2 Channel Sizing") across the wallet bridge, config, boot, api_bridge, snapshot, mining/sync, and node-identity modules. Doc-only. Fixes 2 real staleness bugs found along the way: - node/state.rs: doc comment called drive_popow_bootstrap/ handle_inbound_popow_proof "both follow-up commits" -- both are already implemented (sync_tick.rs, messaging.rs). - api_bridge/tests.rs: a monitoring-scraper note said a stale-field bug was fixed "Pre-r5" -- restated as the direct current-behavior guarantee without the version tag, since no such tag is used elsewhere in the repo. Preserved: every Scala/REST-compat citation, the crate's own stable Mode 1-6 operational-mode taxonomy and R1/R2/R5 Scala-parity validation codes, mainnet-incident rationale (h=28662 sign-flip, silent-stall and header-only-reset-stall bugs), and all consensus/sync-safety documentation (prune-sentinel gating, split-brain best_full_block_height sync, NiPoPoW resume-state classification). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi * fix(ergo-api): regenerate OpenAPI golden fixtures after doc cleanup utoipa embeds doc comments directly into the generated OpenAPI spec, so the internal-artifact citations stripped from wallet/v1 doc comments in 46cdb66 changed the generated native and v1 specs, drifting them from the checked-in golden fixtures. CI caught this (openapi_native_matches_snapshot and openapi_v1_matches_snapshot both failing on all three platforms). Regenerated both fixtures via the documented `regenerate` test target; no other change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Jul 16, 2026
Merged
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
Splits four oversized
ergo-compilerfiles intosubmodule/directories along each file's own natural phase boundaries. Pure move — no logic changes.typer/assign.rstyper/assign/{mod,simple_arms,apply,method_call_like,lower_method,arith_bitop,harness}.rs+expr_contains_untyped_noderelocated totyped.rsemit.rsemit/{mod,scope,dispatch,select,method_call,types}.rscse.rscse/{mod,key,interner,intern,gate,materialize,codec}.rstree.rstree/{mod,assemble,v0_gate,lambda_gate,walk,cast_fold}.rsEach split kept a matching precedent in mind (
typer/mod.rsalready splitsassign/methods/predef_ir/unify) and followed the same discipline: promote crossed-boundary items topub(crate), keep every originally-pubitem's exact external visibility, and — where a file has one bigimplblock (emit.rs'sScope,cse.rs'sInterner) — split it into multipleimplblocks across files rather than trying to force everything into one.Verification
ergo-compilerlib tests, 0 failed.cargo test -p ergo-compiler(andcargo build) run after every single extraction, not just at the end.cargo fmt --all -- --check,cargo clippy --workspace --all-targets --all-features -- -D warnings,cargo test --workspace(294 test-result blocks, 0 failed,ergo-compilerlib still exactly 803/0).pub(crate)/fmt-wrapping), checked for dropped/duplicated items, and re-ran the full gate itself rather than trusting the work in-session.Two non-obvious things the reviews specifically caught and confirmed fixed:
cse.rs:Interner/SymIdwere genuinely part of the crate's external public API (pub mod cse;+ plainpub struct/pub fn). A first-passpub(crate) use interner::*;/pub(crate) use key::*;re-export silently downgraded their external reachability — caught viaclippy --all-targetsreportingdead_codeon methods that were actually externally reachable before. Fixed to genuinepub use.tree.rs:crate::foldimportscrate::tree::{in_fold_range, FoldWidth}directly (a real cross-module dependency, not just a doc reference). Confirmed this path still resolves afterin_fold_range/FoldWidthmoved intotree/cast_fold.rs.Marked as draft — this branch may pick up further similar splits before going up for review; will flip to ready once complete.
Test plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace(294 test-result blocks, 0 failed)git show HEAD:<file>🤖 Generated with Claude Code
https://claude.ai/code/session_01Kw7uCwqmiGna8fpg6TvxGi
Summary by CodeRabbit
New Features
Bug Fixes
Tests