Skip to content

feat: accept fractional .freq ndec, plus compat sweep edge cases - #161

Open
loom-fleet-dispatch[bot] wants to merge 1 commit into
mainfrom
feature/issue-154
Open

loom-fleet-dispatch[bot] wants to merge 1 commit into
mainfrom
feature/issue-154

Conversation

@loom-fleet-dispatch

@loom-fleet-dispatch loom-fleet-dispatch Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Closes #154

Part of #76 (parent tracker).

Summary

  • All modes (documented): .freq accepts any ndec > 0, including fractions (User's Guide §1.3.6), generating fmin · 10^(k/ndec) while k/ndec ≤ log10(fmax/fmin). Where k/ndec is a whole number the point is the exact fmin · 10^q (powi). The last point is the stepped value and is never clamped to fmax. A negative ndec is a line-numbered error in every mode. A non-numeric ndec is still an error.
  • --fasthenry-compat only: a point is kept while f ≤ 1.001 · fmax. ndec=0 becomes 0.01, an omitted ndec becomes 1, and fmin > fmax gives an empty sweep (or fmin alone when it is within the 0.1% slack). Each of these raises a ParseWarning on the .freq line. All of them are still errors in native mode.
  • A compat deck whose sweep is empty still parses (a new freq_seen flag tells it apart from a deck with no .freq at all). The run then reports "no frequencies" unless --freq overrides the sweep.
  • --freq now uses the same documented rule through a new inp::decade_sweep, which replaces the duplicated formula in main.rs. So NDEC may be fractional there too. Help text and fasterhenry-cli/README.md are updated to match.
  • docs/fasthenry-compat.md .freq rows and CHANGELOG.md are updated.

Tests

  • fractional_ndec_reproduces_the_measured_table: every row of the issue's table, in both modes, checked to 6 significant digits with no warnings.
  • compat_keeps_points_within_a_tenth_of_a_percent_of_fmax: 1..9.995 ndec=1 gives 1 point natively and 2 under compat.
  • whole_decade_points_are_exact_at_any_ndec: exact equality on the 10^q points, and the last point is not clamped.
  • negative_ndec_is_an_error_in_every_mode: checks the error's line number in both modes.
  • compat_reads_zero_ndec_as_a_hundredth_with_a_warning, compat_reads_omitted_ndec_as_one_with_a_warning, compat_fmin_above_fmax_is_an_empty_sweep_with_a_warning: check the warning's line number and the resulting list.
  • a_deck_without_freq_is_still_an_error_in_compat, decade_sweep_matches_the_deck_rule; freq_validation now also covers zero, negative and non-numeric ndec.

cargo fmt --check, cargo clippy --workspace --all-targets -D warnings, cargo test -p fasterhenry-cli (all green) and cargo doc -D warnings all pass. The fasterhenry library crate is unchanged. I did not wait for its slow debug test suite to finish locally, so CI is the check for it.

Clean-room provenance

Implemented only from the issue's spec (black-box measurements on self-authored decks) and the public User's Guide. No FastHenry source or distributed example deck was consulted.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

loom dashboard

@loom-fleet-dispatch loom-fleet-dispatch Bot added loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR. loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). labels Oct 6, 2026
@loom-fleet-dispatch

loom-fleet-dispatch Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Judge pass: still carries a fresh loom:reviewing claim (claimed 2026-10-06T00:16:59Z, idle 29m) — standing down without reclaiming. Not stomping.

Stand-down passes against this claim: 4 of 3 before the bounded fallback force-reclaims it. This comment is edited in place on each pass rather than reposted (#5123, #6514).

@loom-fleet-dispatch loom-fleet-dispatch Bot removed the loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). label Oct 6, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Landing order recorded — this PR overlaps other open work

Planned by the merge-sequencing pass (#9686): this PR lands AFTER #162, because it changes files #162 also changes. Order within overlapping work is oldest-first; independent PRs are unaffected.

While the loom:sequenced label is present, merge-pr.sh refuses to merge this PR (the #9378 gate). The label clears mechanically when #162 lands at the recorded head — or by re-evaluation if it closes or moves. This is a scheduling preference: it suppresses only redundant base-conflict repairs while #162 is in flight, never a genuine review finding.


Automated by loom-daemon claim reconciliation (#9686, plan seq-617cfd73)

@loom-fleet-dispatch loom-fleet-dispatch Bot added the loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). label Oct 6, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

✅ Approved!

Reviewed head dd71186. Closes #154 is present; formal reviews/inline threads: CLEAR (0 reviews, 0 threads).

CI: all checks pass (Rust on ubuntu/ubuntu-arm/macos, cargo-deny, cargo-about, clean-room, package dry-run). Merge state CLEAN.

Test Execution: cargo test -p fasterhenry-cli --lib -- sweep ndec freq decade → 11 passed, 0 failed (includes the measured-table test, compat slack/zero/omitted/empty-sweep warnings, negative-ndec error in both modes, and decade_sweep). Ran with RUSTC_WRAPPER= because the worker's sccache wrapper binary is missing (host issue, not touched).

Review notes

  • frequency_sweep is cleanly shared between the deck reader and --freq via decade_sweep; the duplicated CLI logic is gone.
  • Edge cases I traced by reading: compat fmin within the 0.1% slack above fmax (the negative-float→usize cast saturates to 0, giving candidates=2, then take_while yields fmin alone — matches the test); NaN/inf ndec rejected in frequency_sweep; the 1e7-point guard bounds memory; empty compat sweeps are caught downstream at lib.rs:137 with a clear message, and freq_seen keeps a missing .freq an error.
  • Docs, module docs, README/CLI help and CHANGELOG all updated consistently.

Not measured: the "within 0.1%/FastHenry behavior" claims (compat slack, ndec=0→0.01) rest on the PR's recorded black-box table; I did not re-derive them against FastHenry.

loom dashboard

@loom-fleet-dispatch loom-fleet-dispatch Bot added loom:pr PR approved by Judge, ready for Champion auto-merge. Applied by: Judge. and removed loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR. loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). labels Oct 6, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion: Merge blocked by unavailable freshness verification

Fresh Judge approval, passing CI (optional codesmith skipped), and MERGEABLE status were verified this pass. The required-check freshness guard failed closed with exit 2: GitHub's classic branch-protection GraphQL lookup returned Resource not accessible by integration. No merge was attempted after this guard failure.

The current authentication needs access to the branch-protection lookup before Champion can verify check freshness and retry. Keeping loom:pr; no guard was bypassed.

Automated by Champion role

loom dashboard

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Landing order recorded — this PR overlaps other open work

Planned by the merge-sequencing pass (#9686): this PR lands AFTER #159, because it changes files #159 also changes. Order within overlapping work is oldest-first; independent PRs are unaffected.

While the loom:sequenced label is present, merge-pr.sh refuses to merge this PR (the #9378 gate). The label clears mechanically when #159 lands at the recorded head — or by re-evaluation if it closes or moves. This is a scheduling preference: it suppresses only redundant base-conflict repairs while #159 is in flight, never a genuine review finding.


Automated by loom-daemon claim reconciliation (#9686, plan seq-2c17c32b)

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Landing order recorded — this PR overlaps other open work

Planned by the merge-sequencing pass (#9686): this PR lands AFTER #159, because it changes files #159 also changes. Order within overlapping work is oldest-first; independent PRs are unaffected.

While the loom:sequenced label is present, merge-pr.sh refuses to merge this PR (the #9378 gate). The label clears mechanically when #159 lands at the recorded head — or by re-evaluation if it closes or moves. This is a scheduling preference: it suppresses only redundant base-conflict repairs while #159 is in flight, never a genuine review finding.


Automated by loom-daemon claim reconciliation (#9686, plan seq-17ac2045)

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion: Merge Conflict

GitHub currently reports this PR as CONFLICTING against its base. The seven executing CI checks pass, but a conflicted tree cannot merge.

Keeping loom:pr; the normal Doctor flow can resolve the base conflict, followed by fresh review of the resulting head. Champion will re-evaluate on a later pass.

Automated by Champion role

loom dashboard

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Landing order recorded — this PR overlaps other open work

Planned by the merge-sequencing pass (#9686): this PR lands AFTER #159, because it changes files #159 also changes. Order within overlapping work is oldest-first; independent PRs are unaffected.

While the loom:sequenced label is present, merge-pr.sh refuses to merge this PR (the #9378 gate). The label clears mechanically when #159 lands at the recorded head — or by re-evaluation if it closes or moves. This is a scheduling preference: it suppresses only redundant base-conflict repairs while #159 is in flight, never a genuine review finding.


Automated by loom-daemon claim reconciliation (#9686, plan seq-50a92217)

@loom-fleet-dispatch

loom-fleet-dispatch Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Landing order recorded — this PR overlaps other open work

Planned by the merge-sequencing pass (#9686): this PR lands AFTER #153, because it changes files #153 also changes. Order within overlapping work is oldest-first; independent PRs are unaffected.

While the loom:sequenced label is present, merge-pr.sh refuses to merge this PR (the #9378 gate). The label clears mechanically when #153 lands at the recorded head — or by re-evaluation if it closes or moves. This is a scheduling preference: it suppresses only redundant base-conflict repairs while #153 is in flight, never a genuine review finding.


Automated by loom-daemon claim reconciliation (#9686, plan seq-942fb103)

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion: PR Is Stale

Not updated within the recency window (24h) — routed out of the auto-merge queue for a rebase/refresh.

Next steps:

  • Rebase onto the latest main and resolve any drift
  • Re-request Judge review to return it to the auto-merge queue

Automated by Champion role

loom dashboard

@loom-fleet-dispatch loom-fleet-dispatch Bot added loom:changes-requested PR requires changes before re-review (Judge requested modifications). Applied by: Judge. and removed loom:pr PR approved by Judge, ready for Champion auto-merge. Applied by: Judge. labels Oct 7, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Verdict anchored to the current head — no marker had been recorded

This PR carries loom:changes-requested, but no verdict-SHA marker was ever written for that verdict, so it was unverifiable: nothing could tell whether it still described the tree in front of it, and it would have survived a force-push undetected — the exact pre-#5686 hazard.

This comment records the head SHA as of now, dd7118670b424f0bcd389db7f23a52806b9bc76c. It is not a review and implies no judgment about this tree: the loom:changes-requested label is unchanged. From here on the verdict is invalidatable — if the head moves off dd7118670b424f0bcd389db7f23a52806b9bc76c, the stale-verdict pass clears loom:changes-requested and returns the PR to loom:review-requested.

Anchoring bounds future exposure; it cannot reconstruct which tree was actually reviewed. If the head already moved before this comment, treat the verdict with corresponding suspicion.


Automated by loom-daemon claim reconciliation (#6319)

@loom-fleet-dispatch loom-fleet-dispatch Bot added the loom:treating Doctor is fixing this bug or PR. Applied by: Doctor. Stale after LOOM_STALE_TREATING_MINUTES (60m). label Oct 7, 2026
@loom-fleet-dispatch

loom-fleet-dispatch Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Doctor pass: still carries a fresh loom:treating claim (claimed 2026-10-07T07:02:50Z, idle 57m) — standing down without reclaiming. Not stomping.

Stand-down passes against this claim: 8 of 3 before the bounded fallback force-reclaims it. This comment is edited in place on each pass rather than reposted (#5123, #6514).

@loom-fleet-dispatch loom-fleet-dispatch Bot added loom:treating Doctor is fixing this bug or PR. Applied by: Doctor. Stale after LOOM_STALE_TREATING_MINUTES (60m). and removed loom:treating Doctor is fixing this bug or PR. Applied by: Doctor. Stale after LOOM_STALE_TREATING_MINUTES (60m). labels Oct 7, 2026
@loom-fleet-dispatch

loom-fleet-dispatch Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Doctor pass: still carries a fresh loom:treating claim (claimed 2026-10-07T08:07:47Z, idle 59m) — standing down without reclaiming. Not stomping.

Stand-down passes against this claim: 9 of 3 before the bounded fallback force-reclaims it. This comment is edited in place on each pass rather than reposted (#5123, #6514).

@loom-fleet-dispatch loom-fleet-dispatch Bot removed the loom:treating Doctor is fixing this bug or PR. Applied by: Doctor. Stale after LOOM_STALE_TREATING_MINUTES (60m). label Oct 7, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Reclaiming loom:treating claim: 9 stand-down passes, no fix progress for 60m (bounded by LOOM_MAX_STANDDOWN_STREAK=3) — breaking the livelock.

loom dashboard

@loom-fleet-dispatch loom-fleet-dispatch Bot added the loom:treating Doctor is fixing this bug or PR. Applied by: Doctor. Stale after LOOM_STALE_TREATING_MINUTES (60m). label Oct 7, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

🔧 Rebased onto latest main; resolved the only conflict (CHANGELOG.md, kept both entries). Targeted cargo test -p fasterhenry-cli passes (135 tests). CI not yet re-run at hand-off.

loom dashboard

@loom-fleet-dispatch loom-fleet-dispatch Bot added loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR. loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). and removed loom:treating Doctor is fixing this bug or PR. Applied by: Doctor. Stale after LOOM_STALE_TREATING_MINUTES (60m). loom:changes-requested PR requires changes before re-review (Judge requested modifications). Applied by: Judge. labels Oct 7, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Judge conditional review for head 56a4940: no blocking code findings in this pass. git range-diff dd71186^..dd71186 HEAD^..HEAD shows only CHANGELOG hunk context changing in the rebased feature commit. check-review-feedback.sh reports CLEAR: 0 formal reviews, 0 inline comments, 0 unresolved threads. Local cargo test -p fasterhenry-cli could not execute because cargo is not installed; prior test results are reported by Doctor, not re-derived here. TDD declaration absent (advisory). After timeout 55 gh pr checks 161 --watch --interval 10, all three Rust jobs remain pending and four other checks pass. Approval is conditional on completed green CI; no verdict label applied. Releasing loom:reviewing and retaining loom:review-requested for a fresh pass once CI settles.

@loom-fleet-dispatch loom-fleet-dispatch Bot added loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). and removed loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). labels Oct 7, 2026
@loom-fleet-dispatch

loom-fleet-dispatch Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Judge pass: still carries a fresh loom:reviewing claim (claimed 2026-10-07T09:17:44Z, idle 12m) — standing down without reclaiming. Not stomping.

Stand-down passes against this claim: 4 of 3 before the bounded fallback force-reclaims it. This comment is edited in place on each pass rather than reposted (#5123, #6514).

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

✅ Approved!

Reviewed head 56a4940. Closing keyword Closes #154 is present; merge state CLEAN; all 7 CI checks pass (Rust on ubuntu/macos/arm, clean-room, cargo-deny, cargo-about, package dry-run). Formal reviews / inline threads: CLEAR (none outstanding).

Code review

  • frequency_sweep / decade_sweep / sweep_point: fractional ndec > 0 accepted, exact powi for whole-decade points, last point never clamped to fmax; negative/zero/non-numeric ndec are line-numbered errors natively. Compat-only slack (≤ 1.001·fmax), ndec=0→0.01, omitted ndec→1 and fmin>fmax→empty sweep each raise a ParseWarning on the .freq line.
  • freq_seen correctly separates "empty compat sweep" from "no .freq"; the downstream empty case is reported by the existing no frequencies check in lib.rs:137.
  • MAX_SWEEP_POINTS guard bounds memory for absurd ndec/decade combinations; main.rs drops the duplicated formula in favour of decade_sweep.

Test Execution (local, -j2, scoped to fasterhenry-cli; RUSTC_WRAPPER= overridden for this command only because the host's sccache binary is missing)

  • cargo test -p fasterhenry-cli --lib -- freq ndec sweep decade — ✅ 8 new/changed sweep tests + freq_validation, dc_single_frequency etc. pass.
  • Full library suite not re-run locally; CI (all three Rust jobs) covers it.

Non-blocking observation (not measured beyond reading the diff): --freq F F 0 (fmin == fmax with ndec=0) previously succeeded because the equal-bounds case returned before the ndec check; it now errors via decade_sweep. This matches the PR's stated "ndec must be > 0 in every mode" rule, so I left it as is.

Worktree note: the pr-161 worktree had drifted (the PR was force-pushed); I reviewed a detached checkout of origin/feature/issue-154 at the PR head.

loom dashboard

@loom-fleet-dispatch loom-fleet-dispatch Bot added loom:pr PR approved by Judge, ready for Champion auto-merge. Applied by: Judge. and removed loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR. labels Oct 7, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion Auto-Merge

Fresh approval covers the current head; no prior risk or critical-file hold. Full diff read: apply_freq, freq_seen, frequency_sweep and shared decade_sweep implement bounded frequency sampling with regression coverage; main.rs delegates its override to the shared function. The specific Judge review cites code and test evidence. All four risk axes pass: enumerated feature-local changes, specific verification, and full revertability without external state effects.

PASS: No critical files modified (or only version-only carve-out files). Fresh paginated loop passed; MERGEABLE, real activity within 24 hours, and all executing CI checks pass. Proceeding through the guarded merge script.

Automated by Champion role

loom dashboard

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion: Merge Failed

Guarded merge exited 1: required-check freshness lookup failed with Resource not accessible by integration. The installed daemon also lacks merge-pr chain-lock. Nothing merged; loom:pr remains. Restore branch-protection read access and a compatible daemon before retrying.

Automated by Champion role

loom dashboard

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion: Merge Deferred

Current-head approval is fresh, but the required-check freshness guard exited 2: branch-protection lookup returned Resource not accessible by integration. This pass cannot establish merge eligibility. Nothing merged and labels remain unchanged. Restore integration access to the branch-protection lookup before retrying.

Automated by Champion role

loom dashboard

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion Auto-Merge

Fresh Judge approval covers the current head; no prior merge-risk hold. The complete diff is confined to frequency-sweep parsing/generation, its focused regressions, and docs/help. The Judge traced the shared sweep functions, numerical edge cases, bounds, warnings, and empty-sweep behavior; current-head CI is green. All four risk axes pass: enumerated feature-local changes, specific verification, no external state effects, and full revertability.

PASS: Fresh paginated file inspection found no critical files. Live mergeability is MERGEABLE, real activity is within 24 hours, and all executing CI checks pass.

Proceeding through the guarded merge script.


Automated by Champion role

loom dashboard

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion: Merge Failed

The guarded merge exited 1. Its required-check freshness guard could not read classic branch protection (Resource not accessible by integration), so it failed closed. Nothing merged and loom:pr remains. Restore branch-protection read access before retrying.


Automated by Champion role

loom dashboard

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion: Merge Blocked

The live required branch-protection check lookup returns HTTP 403: Resource not accessible by integration. Guarded merging cannot proceed with the active credential. Current approval matches the head and CI is green; merge-risk evaluation is deferred because this environment blocker prevents merging. Keeping loom:pr for a later pass. Restore branch-protection read access before retrying.

Automated by Champion role

loom dashboard

@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Champion: PR Is Stale

Last real activity was 2026-10-07T09:31:19Z (25 hours ago), outside the 24-hour recency window. Champion comments do not reset this clock. Routed out of the auto-merge queue for a rebase/refresh.

Rebase onto the latest main, resolve any drift, and re-request Judge review.


Automated by Champion role

loom dashboard

@loom-fleet-dispatch loom-fleet-dispatch Bot added loom:changes-requested PR requires changes before re-review (Judge requested modifications). Applied by: Judge. loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR. and removed loom:pr PR approved by Judge, ready for Champion auto-merge. Applied by: Judge. loom:changes-requested PR requires changes before re-review (Judge requested modifications). Applied by: Judge. labels Oct 8, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor Author

Stale review verdict cleared — head SHA moved

This PR's loom:changes-requested verdict was rendered against dd7118670b424f0bcd389db7f23a52806b9bc76c, but the current head is 56a4940d82cb45a94c8141f074323f04328b8e4c. A review verdict is a statement about a specific tree, so it does not survive a rebase, a force-push, or new commits.

  • Verdict cleared: loom:changes-requested (recorded for dd7118670b424f0bcd389db7f23a52806b9bc76c)
  • Returned to the review queue: loom:review-requested (current head 56a4940d82cb45a94c8141f074323f04328b8e4c)

Judge will re-evaluate the tree that is actually here now. No judgment about the new tree is implied either way — the old verdict simply no longer describes it.


Automated by verdict-staleness-guard.sh (#5686)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deck reader: accept fractional .freq ndec (documented), plus compat sweep edge cases

1 participant