Skip to content

refactor: continue scaffold desloping#210

Open
wozos wants to merge 82 commits into
logos-co:masterfrom
wozos:wozos/scaffold-deslop
Open

refactor: continue scaffold desloping#210
wozos wants to merge 82 commits into
logos-co:masterfrom
wozos:wozos/scaffold-deslop

Conversation

@wozos

@wozos wozos commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Continuation of the scaffold desloping pass from the existing branch/PR, moved to a wozos-owned branch so the Pi can keep iterating.

This includes:

  • the existing cleanup commits from the current deslop branch;
  • one additional mechanical cleanup pass trimming clippy-style slop and needless allocations/borrows.

Verification

  • cargo test

weboko and others added 3 commits June 6, 2026 20:38
- setup.rs: fix `sha256_hex`. `sha2` is a plain (non-optional) dependency
  and there is no `[features]` table, so `#[cfg(feature = "sha2")]` was
  always false and the FNV-1a fallback compiled instead. The function
  returned a 16-char non-SHA digest, so prebuilt sequencer_service checksum
  verification always failed (and bailed) whenever a `.sha256` was
  published. Use `sha2` unconditionally, matching the project's existing
  hex-encode idiom.
- config.rs: drop the dead `path` binding only consumed by `let _ = path;`.
- run.rs: strip redundant "Step N:" comment prefixes that just duplicate
  the user-facing `[N/total_steps]` progress prints; keep all explanatory
  text.
- wallet_support.rs: collapse stray whitespace in the legacy wallet-config
  warning message.

https://claude.ai/code/session_01KutAyMX3auZUKhuHHp5xXq
- Remove `#[allow(dead_code)] // Fields wired up in later phases` on
  `BasecampAction`; all variants are constructed, so `cargo check` is clean
  without it. The attribute and its placeholder comment were leftover slop.
- Collapse the two-arm `cwd_override` match into
  `as_deref().unwrap_or(project_root)`, matching the idiom the file's own
  tests already use. Behavior unchanged.

https://claude.ai/code/session_01KutAyMX3auZUKhuHHp5xXq
@wozos wozos requested a review from a team June 7, 2026 13:52
wozos and others added 26 commits June 7, 2026 18:20
Two eprintln! warnings (localnet sequencer r0vm hint, run vendored-spel
hint) had runs of ~15-18 literal spaces where line breaks were intended,
printing as garbled single-line text. Switch to the file's backslash
line-continuation idiom so they render as clean single-spaced sentences.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the mechanical clippy warnings flagged across the codebase, all
behavior-preserving:

- config.rs: build OldSchemaMarkers as a struct literal instead of
  Default::default() + field-by-field assignment (field_reassign_with_default).
- setup.rs, process.rs: move #[cfg(test)] modules to the end of the file so no
  real items follow them (items_after_test_module).
- basecamp.rs: array literal instead of vec! for a borrow-only test fixture, and
  separate a trailing doc paragraph from a list item.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`build idl` and `build client` carried byte-identical copies of the
optional-project-path arg parser. Promote the idl.rs copy to
`pub(crate)` and reuse it from client.rs (which already imports from
idl.rs), dropping the duplicate and its now-unused `PathBuf` import.

Behavior-preserving; `cargo build` clean, idl tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two behavior-preserving cleanups in `resolve_selected_programs`,
both aligning with idioms already used elsewhere in deploy.rs:

- replace the `is_none()` guard + `unwrap_or_default()` re-extraction
  with a `let Some(raw) = .. else { return .. }`, matching the
  `let Some(..) = .. else` pattern used throughout the file.
- replace `.iter().any(|p| p == &candidate)` membership test with the
  slice `.contains(&candidate)` it's equivalent to.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three control-flow blocks used the verbose match { Some(x) => x, None => return }
shape where let-else is the dominant idiom in this codebase (report.rs alone
uses it 16 times). Behavior is unchanged.

- template/copy.rs: patch_simple_tail_call_program_id marker lookup
- process.rs: port_open SocketAddr parse
- commands/run_state.rs: tmp file name derivation

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ct_tarball

`extract_tarball` bound `target` from an `if` whose then-branch was `continue`
— a value-from-diverging-branch shape that reads awkwardly and is the only such
case in the tree. Lift the empty-stripped-path check to a guard `continue` and
bind `target` unconditionally afterward. Behavior unchanged; the explanatory
comment stays on the guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The two localnet reset entrypoints triggered clippy::too_many_arguments
while the equivalent command-handler functions in report.rs already
carry #[allow(clippy::too_many_arguments)]. Add the same attribute so
the build is warning-free and the lint handling is consistent across
command modules. No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The other format string in this file (legacy-config warning) uses plain
single quotes inside the double-quoted literal; this one escaped them as
\' for no reason. Unescape to match local style. No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Flatten `else { if in_description { .. } }` to `else if in_description`,
matching the if/else-if chain style already used in the loop and removing
a needless level of indentation. Behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The cache_root resolution bound (bootstrap_cache, _) where the second
slot was always (), and the None arm destructured default_cache_root()
only to re-wrap the path in another ((), ) tuple. Collapse to a plain
let binding reading the path directly. Behavior-preserving.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The doc comment describing reseed_after_wipe (reuses cmd_setup primitives,
byte-equivalent wallet.state, extracted for the byte-equivalence test) sat
orphaned above watch_loop, which it does not describe. Move it onto
reseed_after_wipe, which had no doc.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert the three remaining positional format-arg stragglers
("{}", var) to inline captured-identifier form, matching the
codebase's dominant style (87+ inline usages). Output is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The encoder.finish()?; statement already propagates errors via ? and drops
the returned writer; the let _ = binding was the only such discard-of-?-Ok
in the codebase, inconsistent with the local call()?; idiom used everywhere
else. Behavior unchanged.
The working-tree-dirty branch bound a boolean only to negate it one line
later. Match the literal in the if-let pattern instead, dropping a
nesting level and the throwaway binding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the 5-level if-let/for/if pyramid in collect_api_headers with
let-else + early-continue and a single is_some_and predicate, matching
the let-else / is_some_and idioms already used elsewhere (idl.rs, repo.rs).
Behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Each cloned value is moved or dropped immediately after, so the clone
allocates needlessly:
- basecamp.rs captured_source_row: inline single-use ref_text into detail
- run.rs cmd_run_inner: move resolved into PipelineParams (field shorthand)
- migrate.rs: drop .clone() on basecamp_pin/basecamp_source Options

Behavior unchanged. cargo build + clippy --lib clean, 387 lib tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stency

The two reset-flag parses chained Item::as_value (method reference) into
.and_then(|v| v.as_bool()) (closure), mixing styles within one expression.
config.rs otherwise consistently uses method references for toml_edit
accessors (Item::as_table, Item::as_str, String::as_str). Convert the two
closures to Value::as_bool to match. Behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `curl` not-on-PATH bail passed the constant "logos-blockchain-circuits"
as a positional {} arg. Inline it to match the codebase's inlined-format-args
style; message and behavior are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tyle

The AGENTS_HEADER constant mixed literal source newlines (title break,
bullet list) with redundant `\n\` line-continuations in the middle
paragraph. Since the continuation lines carry no leading whitespace,
`X\n\<newline>Y` produces the same string as a plain literal newline,
so collapsing to literal newlines is behavior-preserving and matches
the rest of the literal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Some(Commands::Create(args)) | Some(Commands::New(args)) duplicated the
Some(..) wrapper; the same file already uses the factored or-pattern form
(ErrorKind::DisplayHelp | ErrorKind::DisplayVersion at cli.rs:593). Use
Some(Commands::Create(args) | Commands::New(args)) for consistency. Flagged
by clippy::unnested_or_patterns. Behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two bail! strings mixed inline captured args with positional `{}`/`{:?}`
for plain local identifiers in a single format string — repo.rs pin
mismatch (resolved_pin/pin/head, alongside inline {label}) and
config.rs check_toml_value (bad, alongside inline {key}/{value:?}).
Inline them to match the codebase's uniform inline-arg convention.
Output unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The write_basecamp_state comment described `project_sources` /
`dependencies` fields as "intentionally ignored ... removed in Phase 3",
but those fields no longer exist on BasecampState and that removal has
already happened. Keep only the accurate note that source lines live in
[basecamp.modules.*].

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_flake_attr

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The comment narrated the removal of `parse_inline_string_array`, `unquote`,
`escape_toml_string`, and the hand-rolled emitter — none of which exist in the
file anymore. It documented nothing in the current code, sitting orphaned
between a function and the test module.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Iterate `for &name in names` instead of `for name in names`, matching
the file's own convention (`for &k in kids`, `for &b in bytes`) and
avoiding a `to_string` call on a `&&str` double reference. No behavior
change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
wozos and others added 7 commits June 8, 2026 19:39
…ommands

cmd_spel was the lone passthrough command-entry taking args: Vec<String>
by value; cmd_client and cmd_idl both take &[String]. The vector is only
borrowed internally (Command::args), never consumed, so this was a sharp
local inconsistency and a needless allocation. Caller updated to pass by
reference. Behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both subcommand arms built the same vec![build, optional-path] via a
.map().unwrap_or_else() that differed only in the called command. Hoist
into a small helper and append the optional path with Option's IntoIterator.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The .map(|p| p.to_string()).unwrap_or_else(|| "unknown".to_string())
idiom was copy-pasted six times across localnet and run. Collapse into a
shared process::pid_text helper using map_or_else.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert single-method closures to method references, matching the
codebase's dominant style (.map(str::trim), .map(str::to_string),
char::is_control). config.rs read_string chained .and_then(Item::as_str)
straight into a closure-wrapped .to_string() in the same expression.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The dominant local idiom for collecting strings is map(str::to_string)/
map(ToString::to_string) (8 sites) over the closure form |x| x.to_string()
(2 sites). Align the two stragglers in localnet log-tail JSON rendering and
the basecamp FakeProbe test fixture. Behavior-preserving.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… idiom

The build_args path-passthrough used .to_string_lossy().to_string(), while
the established idiom for this exact .map(|x| x.to_string_lossy()...) shape in
the codebase (project.rs, basecamp.rs) is .into_owned(). Aligns the closure
and avoids the redundant Cow->String reallocation. Behavior-preserving.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ult::succeeded

Collapse the four repeated `error.is_some() || status.unwrap_or(1) != 0`
warning guards in collect_tool_versions behind a single method, and flatten
the basecamp.modules migration if-let chain via and_then(Item::as_table) to
match the sibling and_then idiom one line above.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wozos wozos force-pushed the wozos/scaffold-deslop branch from d57abd3 to 7474926 Compare June 9, 2026 08:48
wozos and others added 16 commits June 9, 2026 11:33
Both the inline [run].reset and per-profile [run.profiles.*].reset fields
used the same get/as_value/as_bool/unwrap_or(false) chain. Factor it into a
read_bool helper alongside read_string, matching the existing accessor idiom.
Behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lbacks

- basecamp: collapse the six repeated `if field.is_empty()` default-fills for
  [repos.basecamp]/[repos.lgpm] into a shared `fill_repo_defaults` helper.
- run: bind joined paths before canonicalize so the fallback reuses the
  binding instead of recomputing the same `project.root.join(..)` expression,
  matching the `unwrap_or(binding)` idiom used elsewhere.

Behavior-preserving; is_portable_basecamp is pure so eager attr selection is
equivalent. fmt + clippy --all-targets clean; full test suite passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The project overlay tests reinvented temp-dir creation with a manual
SystemTime/process-id naming scheme plus hand-rolled remove_dir_all
cleanup at the end of every test. Every other test module in the crate
(state, process, circuits, repo, doctor_checks, setup, and the sibling
template::skills) uses tempfile::tempdir(), whose TempDir auto-cleans on
drop. Align project.rs to that idiom, which also fixes the temp-dir leak
when a test panics before reaching its manual cleanup.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The git/rustc/cargo probes already iterate; docker/podman were
hand-unrolled copies differing only by tool name. Fold them into the
same loop idiom (format! warning, shared result binding). No behavior
change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_error

The is_connectivity_failure branch (emit JSON error + bail with the
sequencer-unreachable hint) was copy-pasted verbatim across all three
topup steps (preflight, init, pinata claim), differing only in the
message string. Collapse into a connectivity_error helper that returns
the anyhow::Error. Behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The vendored and cache-managed branches each called
sync_repo_to_pin_at_path_with_opts with the same source, pin, and
"lez" args, differing only in target path and sync options. Hoist the
path+options selection into a tuple and make a single call site.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scrub_manifest_text was a one-line wrapper that forwarded verbatim to
scrub_path_string. Inline the callers and delete the alias.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ensure_repo_present and reconcile_repo_source built the identical
`git clone --no-hardlinks -- <source> <path>` command, differing only
by the run_forwarded label. Extract a clone_repo helper so the clone
flags live in one place.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ing)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…arse_failed flag

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pid_is_zombie and pid_command both ran ps -o <field>= -p <pid> with
identical status-check + first-non-empty-line plumbing. Extract that into
ps_field(pid, field) and have both call it. Behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The docker and podman Pass arms were byte-identical save the runtime
name in the detail string. Resolve (runtime, path) first, then build
the CheckRow once. Behavior unchanged: docker still wins when both are
present.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…meContext::command

The wallet-binary Command with the verbose NSSA_WALLET_HOME_DIR env was
copy-pasted across deploy and wallet command sites. Hoist it into a
WalletRuntimeContext::command() method; callers just append subcommand args.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
to_cargo_crate_name (new.rs) and sanitize_file_stem (idl.rs) were the same
character-sanitization algorithm differing only in separator char and empty
fallback. Extract a shared commands::sanitize_separated helper and make both
named functions thin wrappers; behavior and call sites unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…secamp_state

cmd_basecamp_launch and cmd_basecamp_install both loaded basecamp.state,
verified both binaries exist on disk, and bailed with the identical setup
hint. Factor that into one helper. The launch site's extra !is_empty()
match guard was redundant with the .exists() check (an empty path never
exists), so the single helper preserves behavior at both sites.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@weboko weboko force-pushed the wozos/scaffold-deslop branch from 02d66d5 to 249b6ba Compare June 10, 2026 22:28
weboko and others added 2 commits June 11, 2026 00:32
# Conflicts:
#	src/commands/localnet.rs
#	src/commands/new.rs
#	src/commands/spel.rs
#	src/process.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants