Skip to content

The design note's share is the page's, not one constant - #313

Merged
thedavidmeister merged 2 commits into
mainfrom
nd-note-share-per-page
Aug 16, 2026
Merged

The design note's share is the page's, not one constant#313
thedavidmeister merged 2 commits into
mainfrom
nd-note-share-per-page

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Closes #311

The clip was sized for a page nobody asks for

ND_NOTE_BYTES was one constant for every page size, so it had to fit three
rows — and next_design's default page is ONE. The one-row caller paid for two
rows it did not ask for, out of the single field it cannot reconstruct.

The note's share is now derived from the budget and the page actually requested:

limit question.note was
1 (the default) 12,672 2,600
2 5,352 2,600
3 2,912 2,600

MCP_MAX_RESULT_BYTES is unchanged at 36,000. Nothing was taken from another
field: the whole raise is the rows the caller did not ask for. The compile-time
guarantee now holds for EVERY admitted page size rather than only the widest,
and a floor beneath it fails the build if a field added to the row eats the
note's share instead.

3,200 does not compile

The issue's suggestion cannot be a constant. Under the old shape, three rows
times two-for-JSON-escaping left 1,872 bytes of headroom — 312 raw note bytes.
The largest flat value that builds is 2,912, which still clips the 3,046 case,
and it spends the last byte of slack to get there. That is what makes this a
shape problem rather than a number problem.

Measured populations

Org-wide (rainlanguage cyclofinance S01-Issuer), trusted author only, applying
each tool's own predicate to comment bodies. Corpus: the 216 open producer PRs
plus the 500 most recently updated closed ones.

clip cap n p50 p90 max over cap
next_design question.note 2,600 38 889 1,639 3,046 3 (7.9%)
next_ready verdict.note 2,600 297 396 587 937 0
next_close_candidate flag.reason 1,000 111 318 576 1,545 6 (5.4%)
next_close_candidate verdict.note 1,000 27 540 1,459 1,749 9 (33%)
next_leak has.producerNote 2,000 0 not measurable

The seven /ndd rulings the issue names reproduce inside the design row — 989,
1133, 1450, 2335, 2622, 2629, 3046.

next_ready does not share the defect. 297 ready verdicts, none within
1,600 bytes of its cap. Left alone.

next_close_candidate has it worse. A third of its verdict notes clip, and
its two prose fields are the pair a ruling turns on — the flag's claim and the
vetter's check of that claim — so a page carrying one whole and cutting the
other answers half the question. Same treatment, the two fields splitting the
share evenly: 6,152 each at limit: 1, 2,492 at 2, 1,272 at 3.

next_leak could not be measured. The live dashboard reads leaks: 0, so
its population is empty and there is nothing to check the cap against. A proxy —
the newest trusted producer note on each open producer PR, which is the text
leak_reason returns — puts 18 of 109 past 2,000 with a 5,006 maximum, but a
leak note is drawn from a narrower slice of that text than the proxy is, so it
is a reason to re-measure when the queue is non-empty rather than a reason to
change the cap now. NL_NOTE_BYTES is untouched.

The close-candidate figures are a FLOOR. They come from an in:comments search
whose index demonstrably lags — it missed design questions I could read directly
off the PRs — so the real over-cap rate is at least the one above.

Escape

A clipped note now has a cheaper escape than pr_context: ask for a narrower
page. Both tool descriptions and both skills say so.

QA

  • Discriminating tests: a_narrower_page_carries_a_longer_question,
    a_narrower_page_carries_a_longer_claim (both go through next_design_rows /
    next_close_candidate_rows, so the wiring from limit to the clip is what
    they hold, not a hand-passed byte count);
    every_admitted_page_size_keeps_its_note_share,
    every_admitted_page_size_keeps_its_prose_share (each page's share fits the
    budget AND is the whole of what the page leaves — one byte more is over);
    a_page_size_the_limit_refuses_never_reaches_an_unbounded_share (×2);
    a_maximal_page_of_adversarial_rows_still_fits_the_budget (×2) now loops over
    every admitted page size instead of only the widest. Each fails on base
    behavior: base clips at one constant for every page, so the claim that a
    limit: 1 call carries a note the widest page cuts cannot hold there — M1/M6
    re-create exactly that constant inside the new shape and kill the tests.

  • Mutations applied:

    • M1 nd_note_bytes → the old flat constant → a_narrower_page_carries_a_longer_question FAILS
    • M2 next_design_rows passes NEXT_DESIGN_MAX_ROWS instead of limit → same test FAILS
    • M3 nd_note_bytes divides by NEXT_DESIGN_MAX_ROWS instead of rowsevery_admitted_page_size_keeps_its_note_share FAILS
    • M4 nd_note_rows upper clamp dropped → a_page_size_the_limit_refuses_never_reaches_an_unbounded_share FAILS
    • M5 next_design_row's note clip_field removed → a_maximal_page_of_adversarial_rows_still_fits_the_budget FAILS
    • M6 ncc_prose_bytes → the old flat constant → a_narrower_page_carries_a_longer_claim FAILS
    • M7 next_close_candidate_rows passes NEXT_CC_MAX_ROWS instead of limit → same test FAILS
    • M8 ncc_prose_bytes divides by NEXT_CC_MAX_ROWS instead of rowsevery_admitted_page_size_keeps_its_prose_share FAILS

    M3 SURVIVED the first pass — it undersizes the narrow page without breaking
    the budget, and "the share fits" is a claim a smaller divisor also satisfies.
    The maximality half of both page-size tests was added to kill it, and M8 is
    the same mutant on the sibling.

  • Oracle: the byte figures come from GitHub comment bodies read org-wide and
    measured with each tool's own predicate, never from the constants being
    changed. The derived caps come from MCP_MAX_RESULT_BYTES minus every other
    capped field, which the compile-time assertion and the adversarial page test
    check independently of the arithmetic that produced them.

  • Category check: the issue asks for the design clip raised and the three
    siblings checked against their own populations. All four measured; the two
    that showed the defect changed, next_ready cleared on 297 samples, and
    next_leak reported as unmeasurable rather than guessed at.

Summary by CodeRabbit

  • New Features

    • Improved pagination for close-candidate and design queues.
    • Added clearer indicators when fields or comments are truncated.
    • Retrieve larger comment or note content by requesting a single-row page when needed.
    • Page size now adapts more reliably to available response capacity.
  • Bug Fixes

    • Improved handling of large responses and boundary cases to prevent excessive output.
    • Added safeguards for safe page sizing and consistent row limits.
  • Documentation

    • Updated command guidance to explain truncation behavior and content retrieval options.

`ND_NOTE_BYTES` was sized for a three-row page, so the one-row caller — the
tool's default, and the only page a ruling reads — paid for two rows it did not
ask for out of the one field it cannot reconstruct. The share is now derived
from the budget and the page requested: 12,672 bytes at `limit: 1`, 2,912 at 3.

`next_close_candidate` carried the same shape and measures worse, so its two
prose fields get the same treatment, splitting the share evenly.

`next_ready` was measured and does not share the defect. `next_leak`'s
population is currently empty and could not be measured; both are unchanged.

Closes #311

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@thedavidmeister, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 44b16664-bdc4-4f36-82a3-f68e57c4585e

📥 Commits

Reviewing files that changed from the base of the PR and between 00f8af2 and 5134628.

📒 Files selected for processing (1)
  • plugins/human-fsm/commands/ndd.md

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f98ede3d-fa56-4d76-afe2-846b0ec87f42

📥 Commits

Reviewing files that changed from the base of the PR and between 6d0b7a4 and 00f8af2.

📒 Files selected for processing (5)
  • .claude-plugin/marketplace.json
  • plugins/human-fsm/.claude-plugin/plugin.json
  • plugins/human-fsm/commands/ncc.md
  • plugins/human-fsm/commands/ndd.md
  • pr-review-report-rs/src/main.rs

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.


Walkthrough

The PR adds page-aware prose budgets for close-candidate and design queues. It separates row construction from fetching, adds budget and clipping tests, updates truncation guidance, and increments the human-fsm plugin version to 0.25.0.

Changes

Queue pagination and truncation

Layer / File(s) Summary
Close-candidate page-aware rows
pr-review-report-rs/src/main.rs
Close-candidate rows now derive reason and note limits from the requested page size. The caller fetches coverage before pure row construction. Tests cover clipping, invalid sizes, page limits, and document budgets.
Design queue page-aware notes
pr-review-report-rs/src/main.rs
Design rows now pass the requested page size into row construction. Question-note limits derive from non-note fields and page budgets. Tests cover clipping, invalid sizes, minimum shares, and adversarial budgets.
Tool guidance and plugin release metadata
pr-review-report-rs/src/main.rs, plugins/human-fsm/commands/ncc.md, plugins/human-fsm/commands/ndd.md, .claude-plugin/marketplace.json, plugins/human-fsm/.claude-plugin/plugin.json
Tool and command descriptions document page-dependent truncation and single-row retrieval. Both plugin manifests use version 0.25.0.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 00f8a

The change makes note capacity depend on the requested page size and updates the affected tools while preserving the overall result budget; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant Reviewer
  participant next_close_candidate
  participant next_design
  participant RowBuilders
  participant ContextTools
  Reviewer->>next_close_candidate: request page with limit
  next_close_candidate->>RowBuilders: build bounded close-candidate rows
  RowBuilders-->>Reviewer: return rows with truncation indicators
  Reviewer->>next_design: request page with limit
  next_design->>RowBuilders: build bounded design rows
  RowBuilders-->>Reviewer: return rows with truncation indicators
  Reviewer->>ContextTools: request complete prose when truncated
  ContextTools-->>Reviewer: return expanded context
Loading

Possibly related PRs

Suggested labels: ai:ready

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change from fixed clipping constants to page-size-dependent note shares.
Linked Issues check ✅ Passed The PR addresses issue #311 by deriving design-note capacity from page size and evaluating sibling tools against their measured populations.
Out of Scope Changes check ✅ Passed The implementation, tests, documentation, and version updates support the page-size-dependent clipping objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nd-note-share-per-page

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@thedavidmeister
thedavidmeister merged commit 41eab8c into main Aug 16, 2026
23 checks passed
thedavidmeister pushed a commit that referenced this pull request Aug 16, 2026
… moves with the protocol

`command_contract` learned the dispatcher/agent split in #322, so the five
`marketplace_tests` this branch was red on now pass: 1404 green with these
plugin files on that contract.

Three conflicts, all the same shape: #313 edited command bodies this branch
replaces with dispatchers. Resolved to the dispatcher — but taking it alone
would have DROPPED #313, because `agents/*.md` were forked from the command
bodies before #313 landed and carry the truncation mention without the escape it
added. So the escape is ported to where the protocol now lives:

- `agents/ncc.md` — `flag.reason` / `verdict.note` share is the PAGE's, re-ask
  step 1 at `limit: 1`
- `agents/ndd.md` — same for `question.note`

`agents/nr.md` needed nothing: #313 measured `next_ready` over 297 samples, none
within 1,600 bytes of the cap, and left it alone.

`plugin.json` took this branch's description, which is the same text plus the
sentence explaining why the four reads dispatch.

Also deleted the `Landing note (#316)` block from the plugin README. It said
`command_contract` does not yet know this shape; #322 merged, so it is false.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thedavidmeister pushed a commit that referenced this pull request Aug 16, 2026
The version gate asks for a bump PAST main, not a bump from the branch's own
base. This branch raised 0.24.0 -> 0.25.0 while #313 landed 0.25.0, so after the
merge the plugin files change with no version movement and the gate is right to
refuse. Marketplace listing moves with it — a separate check holds the two equal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

ND_NOTE_BYTES clips design notes 22-29 bytes short, costing a pr_context round trip for the sentence that decides the ruling

1 participant