Skip to content

Fix residual app and test-support lint findings - #796

Merged
leynos merged 1 commit into
mainfrom
harden-lint-app-support-residual-20261022
Oct 28, 2026
Merged

leynos merged 1 commit into
mainfrom
harden-lint-app-support-residual-20261022

Conversation

@leynos

@leynos leynos commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Current state — restacked 2026-10-28

This branch was restacked onto #795's head fc5a4234, which is
now the first consumer above main. New head: 2f4706f6.

The restack was forced by a real defect at the bottom of the stack, not by a
routine refresh. docs/users-guide.md in #812 carried a hand-wrapped paragraph
that mdtablefix --wrap reflows, so make check-fmt failed and the required
build-test check went red on every layer above it. That paragraph is now
reflowed in the layer that owns it, and each descendant was replayed onto the
corrected parent. The only content delta between the previous heads and these is
the intended reflow, verified per layer with git diff --stat.

Every layer was then built independently at its own head with
cargo check --workspace --all-targets --all-features — 5041aa6b, fc5a4234,
2f4706f6, f1ec0587, 5d91c482, 4be08596 all EXIT=0 — so no layer's
result is masked by a later one.

CI is running at these heads. The evidence sections below describe earlier
heads and are history, not current proof.
They are left in place because the
per-layer reasoning still holds, but do not read a green run there as a result
on 2f4706f6.


Current state — restacked 2026-10-27

This branch was restacked onto #812's head 4a7d4e6e, which is now the
first consumer above main. New head: 0814b125, rebased on #795.

The whole train was replayed with the user-wide weave merge driver disabled
(-c core.attributesFile=/dev/null), and each resolution was checked against a
driver-free merge-tree result rather than trusted. Two resolutions were
needed across the train: #796 keeps monotony 1.0.0 from main alongside the
layer's mockable, and #798 folds three module-path references into its own
commit.

Every layer was then built independently at its own head with
cargo check --workspace --all-targets --all-features — 6991be57, 0814b125,
81538da1, 206e7763, de717eb2 all EXIT=0 — so no layer's result is
masked by a later one.

CI is running at these heads. The evidence sections below describe earlier
heads and are history, not current proof.
They are left in place because the
per-layer reasoning still holds, but do not read a green run there as a result
on 0814b125.


Scope

Source remediation for residual app and test-support lint findings that the
app/server layer left behind. Four files change, all behaviour-preserving renames and
const-ification:

  • src/app/builder/core.rs: the shadowing let memory_budgets = memory_budgets?
    becomes let configured_budgets = memory_budgets? at three sites
    (shadow_reuse).
  • src/app/frame_handling/backpressure_tests/mod.rs: per_message,
    per_connection and in_flight become per_message_bytes,
    per_connection_bytes and in_flight_bytes, matching the byte quantities
    they actually carry.
  • src/app/frame_handling/tests.rs: fn new becomes const fn new.
  • src/test_helpers/pool_client.rs: rename two shadowing locals to
    callback_count and connection_state.

Position and dependencies

Layer 2 of 5. Stacked on the codec/shared layer, and the parent of the
examples layer.

Reviewer entrypoints

All four files are small and self-contained; the renames are the whole change.

Validation evidence (head 0814b125)

This branch was restacked onto origin/main = 5a3fbfa9 on 2026-10-23, after
main moved three commits including #794's toolchain bump from
nightly-2026-03-26 to nightly-2026-10-22. The previous head aa69b57 and its
local gate results belong to the retired toolchain and are not current
evidence.

The weave merge driver was disabled for the replay
(-c core.attributesFile=/dev/null), and the replay was checked against a
driver-free merge-tree result rather than trusted.

Hosted, at this head. The build-test lane ran at this exact commit and
passed every step. It executes the repository's own targets, so its result is
per-layer evidence for the format and lint gates rather than a result inherited
from a neighbour:

Step Target or tool
Format make check-fmt
Lint make lint — rustdoc with -D warnings, Clippy with -D warnings, Whitaker --all
Lint Markdown markdownlint-cli2 over **/*.md
Spelling make spelling
Validate Mermaid diagrams make nixie
Workflow contract tests make test-workflow-contracts
Feature-gate compile checks make test-feature-gate
Test and Measure Coverage cargo llvm-cov nextest --workspace, default features and targets

https://github.com/leynos/wireframe/actions/runs/39959664152/job/120434169645

The instrumented run executed 1207 tests, 1207 passed, 0 skipped and
reported 87.79%, a 0.01 pp fall from the committed 87.8% baseline that is
inside the ratchet's tolerance band.
Read from the job log, not inferred from the job conclusion.

That run is narrower than the Make targets — no --all-targets, no
--all-features, no doctests — which is why the local runs below are still
required.

Local. Run sequentially at this exact head with make:

Known baseline failures

None observed at this head.

Deferred work

Remaining source census findings, final lint-configuration adoption, the
testkit argument-count decision and Whitaker --workspace coverage.

Summary by Sourcery

Resolve remaining application and test-support lint findings without changing behavior.

Enhancements:

  • Clarify variable names for configured memory budgets, byte-based backpressure limits, and pooled-client state.
  • Make the test adapter constructor eligible for compile-time evaluation.

References

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review.

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Team
  • Run ID: 4bc08b1f-056a-4b3f-becd-e7e95eeb2c43
📥 Commits

Reviewing files that changed from the base of the PR and between 6036697 and 13d33c7.

📒 Files selected for processing: 4
  • src/app/builder/core.rs
  • src/app/frame_handling/backpressure_tests/mod.rs
  • src/app/frame_handling/tests.rs
  • src/test_helpers/pool_client.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

Summary

  • Rename memory-budget bindings to configured_budgets in three app-builder tests.
  • Use byte-specific local names for the backpressure test budgets.
  • Make TestAdapter::new a const fn.
  • Rename the pool-client callback and connection-state locals to clarify their roles.

These changes address lint findings without changing the described test behaviour. No public API changes are reported.

Validation

The supplied hosted run reports that make check-fmt, make lint and cargo llvm-cov nextest --workspace passed. It reports 1,207 tests run, none skipped, and 87.79% coverage. The run omitted --all-targets, --all-features and doctests; no local results are provided.

Walkthrough

The changes update local names in memory-budget and pool-client tests, make TestAdapter::new a const fn, and preserve existing values, assertions, and control flow.

Changes

Test helper updates

Layer / File(s) Summary
Memory-budget test values
src/app/builder/core.rs, src/app/frame_handling/backpressure_tests/mod.rs
The budget tests use named locals for configured budgets and byte-valued budget inputs. Validation and preservation checks remain unchanged.
Adapter and pool helper setup
src/app/frame_handling/tests.rs, src/test_helpers/pool_client.rs
TestAdapter::new becomes a const fn. Pool-client locals receive clearer names; callback and connection handling remain unchanged.

Priority: Low

Change: Refactor

Merge Risk: Minimal · previous review 13d33

The changes are behavior-preserving and present no concrete merge-blocking risk.

Pre-merge checks: Passed 13 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Testing (Compile-Time / Ui) Warning The pull request introduces Rust compile-time behaviour by changing TestAdapter::new to const fn at src/app/frame_handling/tests.rs:56. The only calls remain runtime calls at lines 117 and 119. … Add a Rust compile-time test for the new const capability. Use a focused const-context assertion, such as a module-level const initialised with TestAdapter::new(...), or add a suitable trybuild fixture and invoke it from the existing co…
Passed Passed checks: 13 passed
Check name Status Explanation
Title check Passed The title accurately summarises the behaviour-preserving lint fixes in the app and test-support files. No roadmap task or issue is identified as the change being implemented, so no additional title re…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Testing (Overall) Passed Pass this check. Treat the diff as non-behavioural: it renames local bindings and changes the private test adapter constructor to const fn. Existing assertions and test logic remain unchanged, and n…
User-Facing Documentation Passed Pass this check. The pull request changes only test and test-support code: it renames local bindings in src/app/builder/core.rs, src/app/frame_handling/backpressure_tests/mod.rs, and `src/test_hel…
Developer Documentation Passed Pass the developer documentation check. The pull request changes only private test code: local-variable renames and TestAdapter::new becoming const. It adds no public or internal architectural bou…
Module-Level Documentation Passed PASS. The pull request changes no module declarations or module documentation. Each affected module already has a clear //! description: src/app/builder/core.rs:1, `src/app/frame_handling/backpres…
Testing (Unit And Behavioural) Passed The pull request introduces no new production behaviour or externally observable workflow. The diff only renames local bindings in existing tests and test support, and changes TestAdapter::new to `c…
Testing (Property / Proof) Passed Require no property test or exhaustive proof. The diff only renames local bindings in four test/support files and changes TestAdapter::new to const fn; it introduces no invariant over inputs, stat…
Unit Architecture Passed Pass this check. The pull request changes only test and test-support code. The diff performs local renames, makes TestAdapter::new const, and preserves the existing calls, mutations, async work, a…
Domain Architecture Passed PASS — The pull request does not introduce a domain-architecture violation. The changes are limited to test code and test-support helpers: core.rs changes occur inside mod tests at lines 219 and 3…
Observability Passed The pull request only changes test-support naming and makes TestAdapter::new a const fn. The diff does not add or alter production operational behaviour, process or service boundaries, throughput,…
Description check Passed The description is related to the changeset. It names the same four files (src/app/builder/core.rs, backpressure_tests/mod.rs, frame_handling/tests.rs, test_helpers/pool_client.rs) and describes their…
Full details: Title check

Explanation

The title accurately summarises the behaviour-preserving lint fixes in the app and test-support files. No roadmap task or issue is identified as the change being implemented, so no additional title reference is required.

Full details: Testing (Overall)

Explanation

Pass this check. Treat the diff as non-behavioural: it renames local bindings and changes the private test adapter constructor to const fn. Existing assertions and test logic remain unchanged, and no new functionality or runtime behaviour requires additional tests.

Full details: User-Facing Documentation

Explanation

Pass this check. The pull request changes only test and test-support code: it renames local bindings in src/app/builder/core.rs, src/app/frame_handling/backpressure_tests/mod.rs, and src/test_helpers/pool_client.rs, and makes the private TestAdapter::new constructor const. The diff introduces no user-facing functionality or behaviour change, so docs/users-guide.md requires no update.

Full details: Developer Documentation

Explanation

Pass the developer documentation check. The pull request changes only private test code: local-variable renames and TestAdapter::new becoming const. It adds no public or internal architectural boundary, abstraction, tooling requirement, build requirement, roadmap item, execplan, ADR, or design decision. No documentation update is required.

Full details: Module-Level Documentation

Explanation

PASS. The pull request changes no module declarations or module documentation. Each affected module already has a clear //! description: src/app/builder/core.rs:1, src/app/frame_handling/backpressure_tests/mod.rs:1, src/app/frame_handling/tests.rs:1, and src/test_helpers/pool_client.rs:1. The diff contains only local renames and one const fn change, so it introduces no module-level documentation failure.

Full details: Testing (Unit And Behavioural)

Explanation

The pull request introduces no new production behaviour or externally observable workflow. The diff only renames local bindings in existing tests and test support, and changes TestAdapter::new to const fn; it preserves the existing assertions, error paths, budget construction, callback increment, and connection handling. Existing tests continue to cover builder budget storage/preservation, backpressure thresholds, codec adapter use, and pooled-client behaviour. The testing check therefore has no unmet requirement.

Full details: Testing (Property / Proof)

Explanation

Require no property test or exhaustive proof. The diff only renames local bindings in four test/support files and changes TestAdapter::new to const fn; it introduces no invariant over inputs, states, orderings, or transitions, and no lemma or proof assumption. Existing table-based tests and assertions remain unchanged in behaviour.

Full details: Unit Architecture

Explanation

Pass this check. The pull request changes only test and test-support code. The diff performs local renames, makes TestAdapter::new const, and preserves the existing calls, mutations, async work, and error handling. It introduces no query, command, dependency, persistence, network, clock, or side-effect boundary. git diff --check also passes.

Full details: Domain Architecture

Explanation

PASS — The pull request does not introduce a domain-architecture violation. The changes are limited to test code and test-support helpers: core.rs changes occur inside mod tests at lines 219 and 341–381; backpressure_tests/mod.rs is cfg(test); TestAdapter::new is a test codec helper; and pool_client.rs contains shared pooled-client test infrastructure. The diff only renames local bindings and adds const to a test constructor. It adds no adapter, transport, persistence, framework, or infrastructure dependency to domain logic.

Full details: Observability

Explanation

The pull request only changes test-support naming and makes TestAdapter::new a const fn. The diff does not add or alter production operational behaviour, process or service boundaries, throughput, retries, resource handling, or failure modes. No new logging, metrics, tracing, or alerts are required by this check.

Full details: Description check

Explanation

The description is related to the changeset. It names the same four files (src/app/builder/core.rs, backpressure_tests/mod.rs, frame_handling/tests.rs, test_helpers/pool_client.rs) and describes their renames and const-ification as behaviour-preserving lint fixes. It also covers restack history and CI evidence, which is noise but does not make it off-topic.

Full details: Testing (Compile-Time / Ui)

Explanation

The pull request introduces Rust compile-time behaviour by changing TestAdapter::new to const fn at src/app/frame_handling/tests.rs:56. The only calls remain runtime calls at lines 117 and 119. The diff adds no compile-time or trybuild test, and repository searches found no const-context assertion for TestAdapter::new. The rename-only changes do not need UI tests.

Resolution

Add a Rust compile-time test for the new const capability. Use a focused const-context assertion, such as a module-level const initialised with TestAdapter::new(...), or add a suitable trybuild fixture and invoke it from the existing compile-error harness. Keep the test specific to TestAdapter::new so it fails if the constructor loses const qualification.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Name each budget; keep its value the same.
Make the test adapter const.
Count the callback, then read its reply.
Give each connection state a clear name.
Let unchanged checks mark the end.

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This behavior-preserving lint cleanup removes shadowing, clarifies byte-valued test variables, and const-qualifies a test constructor across app and test-support code. The supplied validation reports formatting, linting, tests, UI/doc tests, markdown lint, and Nixie checks all passing with no known baseline failures.

File-Level Changes

Change Details Files
Clarify memory-budget test variables without changing builder behavior.
  • Rename shadowing locals to distinguish resolved budgets from the input results.
  • Use the clarified name consistently in three shadow-reuse test cases and assertions.
src/app/builder/core.rs
Make byte units explicit in backpressure test helper variables.
  • Rename message, connection, and in-flight budget locals with _bytes suffixes before constructing byte budgets.
src/app/frame_handling/backpressure_tests/mod.rs
Make the test adapter constructor eligible for compile-time evaluation.
  • Change TestAdapter::new to const fn without altering construction behavior.
src/app/frame_handling/tests.rs
Remove residual shadowing in pooled-client test support.
  • Rename the cloned callback counter used by the async closure.
  • Rename per-connection server state before spawning the handler.
src/test_helpers/pool_client.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos added this pull request to stack #802 October 23, 2026 11:32
@leynos
leynos force-pushed the harden-lint-app-support-residual-20261022 branch from aa69b57 to 13d33c7 Compare October 23, 2026 13:15
codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos marked this pull request as ready for review October 23, 2026 19:21
@sourcery-ai

sourcery-ai Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Sorry @leynos, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 6 days and 2 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T19:22:29.865259Z 13d33c7 Draft marked ready
🔒 Security Review ✅ Completed 2026-10-23T19:23:18.454163Z 13d33c7 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@leynos
leynos force-pushed the harden-lint-app-support-residual-20261022 branch from 13d33c7 to 0814b12 Compare October 28, 2026 00:08
codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos force-pushed the harden-lint-app-support-residual-20261022 branch from 0814b12 to 490b13e Compare October 28, 2026 01:22
codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos force-pushed the harden-lint-app-support-residual-20261022 branch from 490b13e to 2f4706f Compare October 28, 2026 01:38
codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos force-pushed the harden-lint-app-support-residual-20261022 branch from 2f4706f to 15ef88c Compare October 28, 2026 02:32
codescene-access[bot]

This comment was marked as outdated.

Base automatically changed from harden-lint-codec-shared-20261022 to main October 28, 2026 03:03
Rename shadowed bindings in the app builder tests, the frame-handling
backpressure fixture, and the pooled test client so each binding names the
value it now holds. The `pool_client` fixture also retains the newly added
connection counters on main while renaming the accept-loop copy.

Mark the frame-handling test adapter constructor `const`, matching the
other adapter constructors in the crate.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@leynos
leynos force-pushed the harden-lint-app-support-residual-20261022 branch from 15ef88c to 34041d9 Compare October 28, 2026 03:03
@leynos
leynos merged commit 45bf405 into main Oct 28, 2026
11 checks passed
@leynos
leynos deleted the harden-lint-app-support-residual-20261022 branch October 28, 2026 03:20
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.

1 participant