Skip to content

Review Numeric Conversion Boundaries #2243

Description

@josecelano

Goal

Review every temporary numeric-conversion Clippy suppression found by #2158 and give each a
documented outcome: retain with a native, source-specific reason; replace with a clearer or safer
conversion; or fix a demonstrated defect.

Why This Is Needed

The #2158 inventory found 77 numeric-conversion suppressions across multiple packages without a
recorded rationale. Undocumented does not mean wrong: many conversions are mathematically normal
(integer counters averaged as floats, bounded counts encoded into fixed wire widths). The problem is
that the reasoning was never written down, so a future reader cannot tell a legitimate exception
from a latent defect. Each conversion needs a review that records what the value means, what its
bounds are, and why the chosen representation is acceptable.

Review Policy

Apply the same three-question review to every owned conversion:

  1. Is the conversion correct for the value's actual semantics and bounds? Record the invariant that
    makes it so, or the case that breaks it.
  2. Is there a clearer or safer expression (a bounded type, From/TryFrom, a rounding or
    saturating operation, or keeping the float) that does not distort the domain meaning?
  3. If the cast stays, add a native reason stating the invariant; use #[expect] when the lint
    fires on every supported toolchain, otherwise #[allow(..., reason = "...")].

Retaining a suppression with a precise reason is a complete, valid outcome. Do not change behavior
merely to satisfy the lint. Add a focused test only where it protects a stated bound or a changed
behavior.

Scope

In Scope

  • Review and document metric aggregate, protocol wire, and domain conversion boundaries.
  • Deliver the three focused child issues below with a recorded outcome for every owned entry.
  • Update Inventory Existing Clippy Allows #2158 inventory evidence as individual suppressions are documented, narrowed, or removed.

Out of Scope

Subissues

Status values: TODO, IN_PROGRESS, BLOCKED, DONE.

Order Issue Local Spec Status Notes
1 #[To be assigned] - Review metric aggregate conversions docs/issues/open/{number}-{epic}-numeric-conversion-metric-aggregate-review/ISSUE.md TODO Owns 72 metric entries: A080-A087, A113-A114, A143-A154, A170, A178-A222, A224-A227.
2 #[To be assigned] - Review numeric protocol wire conversions docs/issues/open/{number}-{epic}-numeric-conversion-wire-review/ISSUE.md TODO Owns A156 and A171 only.
3 #[To be assigned] - Review domain numeric conversions docs/issues/open/{number}-{epic}-numeric-conversion-domain-review/ISSUE.md TODO Owns A099, A123, and A129.

Delivery Strategy

Create this EPIC and all child issues from one reviewed specification bundle. Create the EPIC GitHub
issue first, then the child issues, and move every approved specification to docs/issues/open/.
Merge the resulting documentation-only PR before implementation. Each child implementation starts
from current develop, uses its own branch and PR, and updates #2158 by inventory ID.

The child issues may proceed independently after their shared conversion policy is agreed. When a
child reveals a repository-wide conversion design decision, stop and create an ADR before continuing.

This bundle references the #2158 inventory, which lands through the #2158 documentation PR. Merge
that PR before this specification PR so every inventory ID cited here resolves in develop.

For each subissue implementation in this EPIC, the default completion policy is:

  1. Run automatic checks (linter all, relevant tests, pre-push checks when applicable).
  2. Run manual verification scenarios and record evidence.
  3. Re-review acceptance criteria after implementation and update verification evidence.
  4. Complete an evidence-based implementation review and record a retrospective or why none is needed.

Phase 1

  • Outcome: review policy agreed; child specifications approved and GitHub issues created.
  • Exit criteria: this specification PR merged with all three child specifications in docs/issues/open/.

Phase 2

  • Outcome: each child reviewed on its own branch and PR; every owned entry retained with a reason,
    improved, or fixed.
  • Exit criteria: no owned suppression lacks a native reason, and Inventory Existing Clippy Allows #2158 inventory evidence is
    updated by ID with each outcome.

Progress Tracking

Workflow Checkpoints

  • Epic spec drafted in docs/issues/drafts/
  • Epic spec reviewed and approved by user/maintainer
  • GitHub epic issue created and issue number added to this spec
  • Subissues created and linked in this spec
  • Subissue statuses kept up to date in the Subissues table
  • For each implemented subissue: automatic checks completed and recorded
  • For each implemented subissue: manual verification completed and recorded
  • For each implemented subissue: acceptance criteria reviewed post-implementation
  • For each implemented subissue: implementation completion review recorded
  • Epic acceptance criteria reviewed and checked off
  • Epic issue closed and spec moved from docs/issues/open/ to docs/issues/closed/

Progress Log

  • 2026-09-15 14:46 UTC - GitHub Copilot - Drafted from Inventory Existing Clippy Allows #2158's three approved-for-review numeric design inputs - Awaiting maintainer review
  • 2026-09-16 12:20 UTC - josecelano - Reframed as a review: undocumented exceptions may be legitimate; each conversion is judged on semantics, alternatives, and then documented with a native reason if retained - Chat decision

Acceptance Criteria

  • All three child issues are created and linked after specification-bundle approval.
  • Each child has a clear non-overlapping inventory ownership boundary.
  • A156 and A171 are owned only by the wire-review child issue.
  • Every owned entry has a recorded outcome: retained with a native reason, improved, or fixed.
  • Every implemented child records automated and manual verification evidence.
  • Inventory Existing Clippy Allows #2158 is updated with the outcome for every owned entry.

Acceptance Verification

AC ID Status (TODO/DONE) Evidence
AC1 TODO Pending approved specifications and GitHub issues.
AC2 TODO Child specification scope sections.
AC3 TODO Wire-review specification scope section.
AC4 TODO Child review tables and source attributes.
AC5 TODO Child issue verification artifacts.
AC6 TODO #2158 inventory reconciliation.

Risks and Trade-offs

  • Presuming a suppression is a defect leads to needless behavior changes. The review policy makes
    retain-with-reason a first-class outcome.
  • A shared helper can hide distinct metric, wire, and domain semantics. Keep ownership with each
    boundary unless a demonstrated common abstraction is warranted.
  • Changing conversion behavior can affect public protocol responses. Require boundary-value tests
    and explicit failure behavior before changing a conversion.

References

Activity

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

Metadata

Metadata

Assignees

Labels

EPICContains several subissues

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions