Skip to content

Fold front-end demand into the one multi-owner claim set - #337

Merged
nbenn merged 6 commits into
mainfrom
321-unified-demand
Sep 29, 2026
Merged

nbenn merged 6 commits into
mainfrom
321-unified-demand

Conversation

@nbenn

@nbenn nbenn commented Aug 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Evaluation demand was expressed two ways depending on who asked: a front-end wrote the per-block visibility$required tri-state, while everyone else went through the board update payload components. This retires the front-end's channel — the blocks it needs are held eager under an owner label like any other consumer's, so core no longer distinguishes its demand from a code export's and the demand axis cannot silently become multi-writer the way required did (the defect behind #320).

The tri-state was doing three jobs. Only evaluation demand folds into the owner-keyed eager sets; the other two are separated rather than absorbed:

The declaration rides callback registration because that is the one moment board_server() holds before any flush without a round trip. A payload applies at priority = -Inf, after the first flush has decided what to construct, so an opening eager set sent that way arrives too late by construction. Seeding it at registration also removes the need to tell "has not declared yet" from "holds nothing": the gate_claimed latch is gone, and the three construction-order tests it protected fail if the opening set is instead made to land at the end of the first flush.

The payload component was sustain and the declaration gate_claim(); both are now eager, since a lazy board evaluates exactly the blocks some owner holds eager. Neither name was released.

Migration

Before After
visibility$required[[id]](TRUE) update(list(eager = list(<owner> = list(add = id))))
visibility$required[[id]](FALSE) update(list(eager = list(<owner> = list(rm = id)))); a built block stays built, and an unbuilt one is left to the background pass
visibility$required[[id]] non-NA makes the board lazy the callback returns eager("<owner>", <opening ids>), on its own or as one element of the list it returns
visibility$visible[[id]](…), visibility$frozen[[id]](…) unchanged

Breaking for blockr.dock, which drives required in mark_cards_built() and show_cards(); blockr.dock#420 adopts it. The neighboring unreleased 0.1.4 NEWS entries (#333, #320, #338) described their changes in terms of the required channel and were corrected in place.

The merge queue checks blockr.dock against its adoption PR rather than main, which cannot start against this branch, and blockr.dag and blockr.assistant against throwaway branches whose Remotes pin that PR, since their e2e tests boot a dock app:

BristolMyersSquibb/blockr.dock#420
BristolMyersSquibb/blockr.dag@321-unified-demand
BristolMyersSquibb/blockr.assistant@321-unified-demand

Fixes #321

@codecov

codecov Bot commented Aug 18, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
R/block-server.R 96.64% <100.00%> (ø)
R/board-server.R 97.58% <100.00%> (+0.59%) ⬆️
R/stack-gate.R 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nbenn

nbenn commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator Author

Core's own front-end now drives the visibility channels through a board callback rather than from inside board_server(), which changes what this PR has to carry. The stack accordion is gate_stacks(), and it is board_server()'s default callbacks value; a board driven by another front-end passes its own callbacks, so core tracks nothing and never competes for the same slots. Dock already does exactly that from blockr_app_server.dock_board(), so nothing is needed on that side. Landed on main in #339.

Worth carrying into the vocabulary here: whatever replaces required, who tracks is then answered by which callbacks the board was given, not by a runtime check inside board_server(). That makes it static, so there is no declaration ordering to get right, and a front-end cannot end up sharing a channel with core by accident.

That callback is a second migration site alongside blockr.dock#417, and a small one — its whole body is one required write and one visible write, and both collapse into a gate declaration plus a sustain claim under the same label. It could not be written that way against main: measured there, a callback that sends only a sustain claim leaves gating_active() at FALSE and every block needed, because activation reads has_required() alone, and the construct test in test-visibility-gating.R pins that a request must not flip an ungated board. Worth a line in the migration table so the core-side consumer is not missed.

One asymmetry to flag, because it runs the other way. The stack callback states what it requires only from an observer, never synchronously as it is set up, and that is fine against required: required[[id]](FALSE) still counts as ever-required, so a collapsed stack's blocks stay in the construction set either way, and nothing evaluates in the flush-1 transient because rendering is gated on visible, still NA. Under a claim set they are simply absent from the claim, so the first tick of construct_blocks_in_background() would take its ungated fast path, build the whole board and destroy the ticker. The synchronous declaration this PR already requires is what covers that, and a core-side owner will need it for the same reason a front-end does — worth saying explicitly, since "state it as you are set up" reads like a front-end-only obligation today.

@nbenn

nbenn commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator Author

On retiring visible as well: I probed an ungated core board on main with the second stack collapsed, and the result argues against that channel rather than for it.

  • The eval status of d, in the collapsed stack, is ready — it evaluated.
  • Shiny's own session$clientData[["output_my_board-block_d-result_hidden"]] is TRUE, so shiny knows the element is hidden.
  • The rendered output is in the DOM anyway.

The reason is the ordering: the card is rendered into #my_board_blocks while visible, then moved into the collapsed accordion body by the move-block-ui message, so the hidden report arrives after the first render and suspendWhenHidden never gets its chance.

So neither shiny's flag nor a front-end's acknowledgement is early enough to stop the first render. The only signal that is early enough is what the front-end has already declared it intends to show — which is demand. Gating rendering on visible works because the acknowledgement carries the same information one round trip later, not because it says anything demand does not. As a render-gate input it looks redundant.

Two jobs I would not fold in with that, because I have not measured them:

  • The background-construction hold. required_fulfilled() is visible's only other consumer in core, and it is already weak — schedule_construction() arms through onFlushed() plus later(), so it waits for a genuine idle window regardless, and this PR's own notes record that production satisfied the claim latch only by accident of the pacing delay. My guess is it drops out. It is a guess.
  • The NA state. Dock reads ls(visibility$visible) with !is.na as its build ledger (built_cards() / slot_built() in R/block-ui.R), which is dock state living in a core channel. Retiring visible means dock keeps its own ledger. That is a dock change rather than a core requirement, but it should be named, or the retirement reads cheaper than it is.

@nbenn

nbenn commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator Author

To state the end point the two comments above only circle: once required is gone here and visible goes the way argued above, the remaining reason to hand a callback the visibility bundle is frozen. Move that too and the bundle stops being part of the callback signature altogether — it becomes core-internal state written by the applied payloads, and update is the whole front-end interface. That seems worth stating as the target, because each channel argued on its own reads like three separate tidy-ups rather than one retirement.

The frozen channel has exactly one consumer in core (block-server.R, the input-freeze guard) and one writer in blockr.dock (freeze_hidden_inputs(), driven off the card's section state and the board lock). Unlike paint it is not redundant: only the front-end knows which controls it has hidden, so core cannot derive it. It moves rather than disappears — an owner-keyed update component of the same shape as sustain, since what it expresses is "this owner asserts these blocks are read-only".

One cost worth naming rather than discovering later: dock already folds two update writers into a single payload per flush, and routing frozen through the same channel makes a third. That is a dock-side change with its own edge cases, not a free relabelling.

The part that genuinely resists is the gate declaration, and it is the reason the bundle cannot simply be deleted. A payload applies at priority = -Inf, after the first flush has decided what to construct, which this PR measured — so the declaration cannot ride update the way demand can. My suggestion is to make it a property of the board rather than a runtime write: a dock_board states that an external front-end drives visibility, core reads it as it sets up, and there is no ordering left to get wrong. Core's own accordion callback then needs no declaration at all, since it is the default and the board says nothing. The alternative — keeping one narrow declaration handle in the callback signature while the three channels leave — works too, but it keeps a foot in the API this is trying to retire.

@nbenn
nbenn force-pushed the 321-unified-demand branch from 780518d to f6e2f8a Compare August 20, 2026 13:15
@nbenn

nbenn commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Heads up from #343 / #344: this branch reintroduces the load-time evaluation that #343 reports.

Measured on 321-unified-demand as of Port core's stack gating onto the claim set, running this branch's own inst/examples/board/gate/app.R with a counter registered on block_eval for dataset_block:

evaluated at load : datasets::BOD datasets::ChickWeight
status_b          : ready
status_d          : dormant

The block in the collapsed stack (c, stack s2) runs once and then parks. The mechanism is the same three lines as on main: gate_stacks() writes vis$gate(owner) inside the observer, only once stacks_reported() is true, so through the first flush gating_active() is FALSE while rv$needed is still its initial TRUE, and everything built is evaluated.

That is a small fix on its own — the seed from #344 ports over — but it points at something this PR comes close to solving and stops just short of.

The declaration is right; the timing is not

Replacing "has anyone written demand" with not_null(vis$gate()) is the fix for the real ambiguity, and it is the part worth keeping: the presence of a front-end becomes something declared rather than inferred from the presence of its data.

What stays inferred is when. Board updates apply at priority = -Inf, after the flush's priority-0 observers, so a front-end whose only channel is update cannot declare in time by construction. The requirement to declare synchronously therefore remains an unwritten convention that every front-end has to know and that nothing enforces — and it is precisely what blocks retiring visibility in favour of the general-purpose update mechanism downstream (BristolMyersSquibb/blockr.dock#417).

The rv$gate_claimed latch is a second symptom of the same gap: it exists because an empty claim and no claim are indistinguishable, which is only a question because core does not know a claim is coming.

Registration is the moment core can know

The callback list is the one thing board_server() holds synchronously, before any flush and without a round trip. Letting a callback declare itself the gate owner there, rather than by sending anything, closes the window without a convention:

Happy to take this on if you would rather it were folded in here than tracked separately.

@nbenn

nbenn commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

This carries into https://github.com/orgs/BristolMyersSquibb/discussions/489 with one change, along the lines of the last comment above: take the gating declaration and the opening claim from the board rather than visibility$gate(). The board answers initial_block_ids() before the first flush, NULL where no front-end gates and otherwise its opening screen, which core seeds as the front-end's claim, so the gate_claimed latch goes with the synchronous write. The generic comes from #350, which is closed as superseded because its build ledger leaves core. Keep this PR to demand: rendering, ordered construction and freezing are #366, #367 and #368.

@nbenn

nbenn commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

Revised request, replacing my comment above. The declaration moves to callback registration rather than initial_block_ids(), and https://github.com/orgs/BristolMyersSquibb/discussions/489 is being revised to match. A board generic is the wrong discriminator: which callbacks a board was given decides who drives demand, as your comment above argues, and a plain board served with another front-end's callbacks would otherwise get core's open-stacks answer, seeded under a claim no front-end manages.

Changes for this PR:

  1. Have the gating front-end declare itself when its callback is registered, together with its opening claim, and have core seed that claim under the callback's owner label before the first flush. That replaces the synchronous visibility$gate() write, and the gate_claimed latch goes, since the opening claim no longer arrives by payload after the first flush has decided what to construct. One shape: the callback returns a declaration of its owner label and opening claim, which core reads as it runs the callbacks at setup. The gate_stacks() callback declares the open stacks' blocks, and dock's the active view's; Claim the on-screen blocks instead of driving core's required channel blockr.dock#420 follows whatever shape this settles on.
  2. Drop construct = setdiff(board_block_ids(board), shown) from claim_shown_blocks(). The construct component builds every id it names in the flush that applies it, so this builds every collapsed block at once, where main paces them through the background pass. Claim the on-screen blocks instead of driving core's required channel blockr.dock#420 measured the same effect and sends no construct for off-screen cards. Leave them to the background pass until Build construct requests in the order given, paced by core, instead of a background pass #367.
  3. Keep the scope to demand. The visible channel, the render gate and gate_fulfilled() stay as they are here, since Render a block when the front-end claims it, not when visible reports it painted #366, Build construct requests in the order given, paced by core, instead of a background pass #367 and Move freezing into update and drop the visibility bundle from the callback signature #368 replace them.
  4. Rebase onto main, which is 14 commits ahead and conflicts, and update the gate row of the migration table.

The per-block `required` channel expressed the same evaluation demand the
`sustain` claims already carry, so core distinguished the front-end from
every other consumer and the channel could quietly become multi-writer.
The front-end now claims under an owner label like anyone else, asks for
bare construction with `construct`, and declares that it drives visibility
by writing that label into the new board-wide `visibility$gate` channel --
inferring gating from claims instead would let a consumer asking about one
block park every other block on an ungated board.
The `gate_stacks()` callback landed on main while this branch was open and
drove `visibility$required` directly. It now claims the blocks of every open
stack under an owner label of its own, holds the collapsed ones built with a
`construct` request, and declares the gate on the first accordion report --
the same moment the old slot writes used to activate gating.
The #343 fix declares what core renders open as the board server is set up,
which the claim set expresses as the gate declaration plus an opening claim.
The declaration stays a synchronous write: a payload only applies at the end
of the flush it is written in, by which time the window it closes has passed.
A payload applies at the end of the flush it is written in, after the
first flush has decided what to construct, so a front-end whose only
channel is `update` could not declare in time. The declaration now rides
the one thing board_server() holds before any flush: a callback returns
gate_claim(owner, blocks), and core seeds that opening claim as it runs
the callbacks. That replaces the synchronous visibility$gate() write and
the gate_claimed latch, which existed only because an opening claim sent
by payload was indistinguishable from none until it landed.

The stack gate declares its open stacks this way and no longer sends a
`construct` for collapsed blocks, which built all of them in one flush
where the background pass paces them.
@nbenn
nbenn force-pushed the 321-unified-demand branch from ab01dd9 to ef1d124 Compare September 28, 2026 14:33
@nbenn

nbenn commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

All four applied, rebased onto current main, and the migration table's gate row updated.

The callback returns gate_claim(owner, blocks), either on its own, as gate_stacks() does, or as one element of the list it already returns for plugins, which is what dock's will need. Core pulls the declaration out before splicing the rest into plugin arguments, seeds blocks as that owner's claim, and validates it as it would a sustain set, so an unknown id fails with the same class. At most one callback may declare. The visibility bundle callbacks receive no longer carries gate, so the declaration is the only way to gate a board.

The gate_claimed latch is gone, and the evidence that it is safe to drop is measured rather than argued: with the seeded claim deferred to the end of the first flush, as a payload would land, the three construction-order tests the latch protected fail, along with a new test pinning that the claim is in place at the first flush. Seeded at registration, all four pass, and so does the stack gate's browser test asserting that only datasets::BOD evaluates at load.

The construct in the stack gate's claim is dropped. Apart from gate_fulfilled() losing the latch, visible, the render gate and gate_fulfilled() are as they were.

One side effect to review: callbacks now run through lapply() rather than an indexed loop, because assigning a NULL result into cb_res[[i]] deleted the element and shifted every later callback's result by one. A callback returning NULL now contributes nothing.

For BristolMyersSquibb/blockr.dock#420, the shape to follow is its callback returning gate_claim(dock_id(session$ns), <active view ids>) as one more element of the list it returns.

Core calls only json_read() and json_write_str(), both in the CRAN
release, and resolving the GitHub remote was the one step in dependency
install that needed the GitHub API.
A board is eager by default and evaluates every block; a front-end makes
it lazy by returning eager(owner, blocks) from its callback, and the
blocks any owner holds eager are what a lazy board evaluates. The same
word now names the payload component, so the `sustain` component becomes
`eager` and gate_claim() becomes eager(), and the docs stop describing
the declaration as driving visibility. Nothing here has been released,
so the rename carries no deprecation.
@nbenn
nbenn marked this pull request as ready for review September 29, 2026 09:29
@nbenn
nbenn added this pull request to the merge queue Sep 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 29, 2026
@nbenn
nbenn added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main with commit 6161b4a Sep 29, 2026
10 checks passed
@nbenn
nbenn deleted the 321-unified-demand branch September 29, 2026 09:57
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.

Evaluation demand should be one multi-owner set, not two channels

1 participant