Repository navigation
Conversation
|
Judge review: approved (closes #144) Checked the diff against each acceptance criterion in #144:
Clean-room: the code, docs and CHANGELOG come from the issue spec and public-guide behaviour. I found nothing derived from FastHenry. CI: all checks were still pending when I reviewed. I judged the code only, and merging must wait for CI to go green and for |
|
Verdict anchored to the current head — no marker had been recorded This PR carries This comment records the head SHA as of now, 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) |
|
Landing order recorded — this PR overlaps other open work Planned by the merge-sequencing pass (#9686): this PR lands AFTER #152, because it changes files #152 also changes. Order within overlapping work is oldest-first; independent PRs are unaffected. While the Automated by loom-daemon claim reconciliation (#9686, plan seq-8c981698) |
|
Landing order recorded — this PR overlaps other open work Planned by the merge-sequencing pass (#9686): this PR lands AFTER #152, because it changes files #152 also changes. Order within overlapping work is oldest-first; independent PRs are unaffected. While the Automated by loom-daemon claim reconciliation (#9686, plan seq-1d039e6e) |
|
Landing order recorded — this PR overlaps other open work Planned by the merge-sequencing pass (#9686): this PR lands AFTER #152, because it changes files #152 also changes. Order within overlapping work is oldest-first; independent PRs are unaffected. While the Automated by loom-daemon claim reconciliation (#9686, plan seq-36cf2910) |
|
Landing order recorded — this PR overlaps other open work Planned by the merge-sequencing pass (#9686): this PR lands AFTER #152, because it changes files #152 also changes. Order within overlapping work is oldest-first; independent PRs are unaffected. While the Automated by loom-daemon claim reconciliation (#9686, plan seq-52da1383) |
|
Champion: Cannot Auto-Merge Cannot auto-merge:
Next steps:
Keeping Automated by Champion role |
02aafa9 to
0e06590
Compare
|
🔧 Resolved merge conflicts with main branch (rebased onto origin/main). Changes:
No substantive code changes. Verified locally: |
|
Stale review verdict cleared — head SHA moved This PR's
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 loom-daemon claim reconciliation (#5686) |
|
🩺 Doctor standing down: PR head moved to |
|
Static review of head 0e06590 found no blocking issue in the units parsing, forward-only scale changes, tests, or documentation. This is conditional review feedback, not approval. Verification: check-review-feedback.sh reported CLEAR (0 formal reviews and 0 inline comments). git rev-list --count HEAD..origin/main returned 0. Local test-plan commands could not execute: timeout reported cargo: No such file or directory, so fmt, clippy, and CLI tests are unverified locally. The PR has no TDD claim (advisory only). After a bounded foreground wait and another gh pr checks snapshot, Rust (macos-latest), Rust (ubuntu-24.04-arm), and Rust (ubuntu-latest) are still pending; the four other CI jobs passed. Releasing loom:reviewing and retaining loom:review-requested for a fresh review once CI settles. |
|
✅ Approved! Reviewed head Verification:
|
|
Standing down: re-checked labels immediately before writing my verdict and found loom:reviewing removed and loom:pr already present — another Judge's approval of head |
|
🔧 Resolved merge conflicts with main branch. Changes:
No substantive code changes made - only conflict resolution. Local: |
|
Judge pass: still carries a fresh Stand-down passes against this claim: 1 of 3 before the bounded fallback force-reclaims it. This comment is edited in place on each pass rather than reposted (#5123, #6514). |
|
Judge code evaluation completed for Validation:
Releasing |
|
Judge pass: still carries a fresh Stand-down passes against this claim: 2 of 3 before the bounded fallback force-reclaims it. This comment is edited in place on each pass rather than reposted (#5123, #6514). |
|
✅ Approved! Reviewed at head 59011cf (local worktree was on a stale pre-rebase copy of the commit, so I re-checked out the actual PR head before testing).
Test Execution
Note: the PR body's coordination note says it may conflict with #148 in |
|
Champion Auto-Merge Current-head Judge approval is fresh; no prior merge-risk hold. Read the full diff: compat_unit_factor handles safe spellings and rejects misread prefixes; apply_units and the implicit metre default apply forward only. Changes are confined to deck parsing or plane bars plus tests/docs, the specific Judge review is backed by current-head CI, and reverting the commit undoes the change. All four risk axes pass. PASS: No critical files modified (fresh paginated list inspected). MERGEABLE; last real activity 1h ago; all seven CI checks pass. Proceeding through the guarded merge script. Automated by Champion role |
|
Champion: Merge Failed Guarded merge exited 1. The required-check freshness lookup returned Automated by Champion role |
|
Champion Auto-Merge Fresh Judge approval covers head PASS: No critical files modified (fresh paginated file list inspected). Live mergeability is MERGEABLE, real activity is within 24 hours, and all required CI checks pass. Proceeding through the guarded merge script. Automated by Champion role |
|
Champion: Cannot Auto-Merge The guarded merge failed closed because the required-check freshness lookup for classic branch protection returned Nothing merged; keeping Automated by Champion role |
|
Champion Auto-Merge Fresh approval covers the current head; no prior risk or critical-file hold. Full diff read: compat_unit_factor, missing-unit initialization, repeatable apply_units and finish validation are bounded to deck units; geometry and conductivity regressions verify forward-only scaling. 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 |
|
Champion: Merge Failed Guarded merge exited 1: required-check freshness lookup failed with Automated by Champion role |
|
Champion: Merge Deferred Current-head approval is fresh, but the required-check freshness guard exited 2: branch-protection lookup returned Automated by Champion role |
|
Champion Auto-Merge Fresh Judge approval covers the current head; no prior merge-risk hold. The complete diff is confined to compatibility-mode unit parsing, its focused regression tests, and docs. The Judge traced unit spelling classification, missing-unit fallback, forward-only repeated directives, and native-mode 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 |
|
Champion: Merge Failed The guarded merge exited 1. Its required-check freshness guard could not read classic branch protection ( Automated by Champion role |
|
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 |
|
Champion: PR Is Stale Last real activity was 2026-10-07T07:42:56Z (27 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 |
|
Stale review verdict cleared — head SHA moved This PR's
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) |
Closes #144
Summary
Everything is under
--fasthenry-compat(ParseOptions::fasthenry_compat); native mode is unchanged.meter(s),metre(s),kilometer(s),kilometre(s),inch,inches(case-insensitive) — are accepted at the documented unit's factor, with a warning on the.unitsline naming the documented spelling. The documented seven (andmil) raise no warning.millimeter*/millimetre*/milli*(FastHenry reads mils, 2.54e-5 m) andmicron*/micrometer*/micrometre*(FastHenry reads metres). The message names the misreading and the documented spelling (mm/um). Anything else (centimeter,nm,ft, …) stays the ordinary unknown-unit error..unitsmeans metres, with a warning on the first line that needs a unit (.default,N,E,G,.hole,.contact), pushed in deck order..unitsapply from their own line onward only. Every value read so far,.defaultfields included, is already stored in metres and S/m, so nothing earlier gets re-scaled. Compat: default an unspecified conductivity to copper (5.8e7 S/m), with a warning #148's copper default is a physical 5.8e7 S/m that ignores the unit, so it composes with this without any change.Coordination
fasthenry_compat: boolfield toDeckBuilder, wired up fromoptionsexactly as Compat: default an unspecified conductivity to copper (5.8e7 S/m), with a warning #148 does. Whichever PR merges second gets a trivial conflict there (same field, same initializer), and probably another in theParseOptions::fasthenry_compatdoc comment, where both PRs append a paragraph.units_compatso it can't collide with Compat: default an unspecified conductivity to copper (5.8e7 S/m), with a warning #148'sparse_compat_warned.compat_mode_ignores_directive_like_first_lineused to expect an error when the swallowed line 1 was.units m. That deck now parses in metres with a warning, and the test checks that instead.Docs
docs/fasthenry-compat.md,.unitssection: I updated the.units <unit>row and added rows for several.units, for long spellings, and for misread spellings. I also added a compat note to the.defaultrow.CHANGELOG.mdhas an Unreleased entry.Test plan
cargo fmt --checkcargo clippy --all-targets --workspace -- -D warningscargo test -p fasterhenry-cli. This includes new tests for long spellings, misread spellings, missing.units, forward-only repeated.units(with a mm-era.default sigma), and native mode staying unchanged.Clean-room: written only from the issue spec and the public docs; no FastHenry source or example decks were consulted.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.