ogar-blockly: the block vocabulary is one byte, a function is 360 of them - #235
Conversation
…them Adds the plug-and-play producer crate for the 0x17XX Blocks domain, following the ogar-obo pattern: concept ids are authoritative here and the shared CODEBOOK keeps zero 0x17XX rows, so only consumers that dep this crate compile a block vocabulary. Three collapses, each retiring a design question rather than answering it. The vocabulary is ONE BYTE. Commands and concepts share a 256-slot palette, so two frontends rendering the same operation land on the same slot -- logic_compare[LT], operator_lt and PaletteOp::LT are one entry. That is where the convergence becomes real instead of nominal. Measured from the two Apache-2.0 sources on disk: Blockly 57 block types / 71 operation codes, scratch-blocks 171 opcodes (59 shared-core, 108 device, 4 menu helpers). The deduplicated union is about 190 and fits 256; the naive sum of the two would have been 330. Content needs ONE classid. Operations are payload bytes rather than concept ids, so the per-operation concept space -- and the 255-slot ceiling an earlier pass computed for it -- does not need to exist. BlockConcept has two variants: Content and Inventory. The body budget is derived, not chosen. value(480)/16 = 30 facet slots, each 16-4 = 12 payload bytes, so OPS_PER_FUNCTION = 360, compile-asserted against the node layout. A longer function is split, never given a wider row -- the canon's "scale is the next cascade level, never field-widening" applied to program structure. It also makes "does this function fit?" checkable before writing rather than a surprise at write time. Storage is an inventory SoA plus N content SoAs partitioned by function. That is the V3 mailbox doctrine rather than a storage preference: one function = one owner = its own SoA, so a registry read never touches a body and no shared table accumulates writers. Palette ranges are prefix-routable. The shared computational core sits below DEVICE_FAMILY_FLOOR = 0x90 and sprite/stage families above it, so "is this operation frontend-specific?" is one compare with no table lookup. The device range is reserved, not allocated -- 108 opcodes were measured and mint on demand. Slot 0x00 is NOP, the zero-fallback, so a partially-filled body carries no length field on the wire. The provenance fence is enforced in the module docs: entries derive only from the Apache-2.0 Blockly and scratch-blocks definitions, never from AGPL scratch-vm. That is what lets a GPL consumer link this public codebook while the GPL boundary stays inside the consumer repo. Both load-bearing falsifiers were verified by breaking what they guard -- an injected palette collision and a cap loosened by one each fail the suite. The second run surfaced a real latent defect: from_ops derived len from the caller's length rather than the copied count, so the guard alone kept ops() in bounds and a future edit to it could have produced an out-of-range len. Now derived from the write count. Ledger: docs/DISCOVERY-MAP.md D-BLOCKS-PALETTE. Gates: 10 tests pass, fmt clean, clippy -D warnings clean, cargo check --workspace --all-targets exit 0, workspace tests clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DN5odWu4eisdevH9bPWyz
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_41caae64-7f9e-4acf-8135-b731aa3e0314) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79c7a815c5
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut body = Self::new(); | ||
| let mut written = 0usize; | ||
| for (slot, op) in body.ops.iter_mut().zip(ops) { | ||
| *slot = op.0; |
There was a problem hiding this comment.
Encode operands alongside each opcode
For blocks with fields or inputs, this copies only the opcode byte and discards the data needed to execute or reconstruct the block. For example, numeric literals 1 and 10 both become PaletteOp::NUMBER, and WAIT(1) and WAIT(10) produce the same body; the documented edges carry caller/callee wiring rather than these values. Any nontrivial Blockly/Scratch program therefore loses semantics when converted to this advertised function-body representation, so operands or references to them must be included in the encoding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and I have no fix for it — flagging that plainly rather than pushing a patch that looks like one.
WAIT(1) and WAIT(10) really do produce identical bodies, and NUMBER says a literal is here without saying which. The docs claimed "one function = one node = up to 360 operations" as though that were a complete program representation. It isn't, and that overclaim is mine.
49deab6 scopes the claim honestly instead of papering over it: FunctionBody is now documented as an opcode skeleton — the vocabulary layer of a program, not a program — with an explicit "what this does NOT yet encode" section, on both the crate and the type.
Choosing the operand encoding is an architecture decision rather than an implementation detail, so it goes to the operator, not into a drive-by commit. The two candidates are named in the docs so it gets decided deliberately:
- Immediates in the stream — some palette entries are followed by operand bytes (constant-pool index, variable slot). Keeps 360 as a bound on bytes, lowers the effective op count, and needs a home for the pool.
(opcode : operand)pairs — read each 12-byte lane as the LE contract's L46 × (u8:u8)rail: 6 pairs per facet, 180 per node. Operands become palette-addressed in the same byte space at the cost of halving the op budget, using a sanctioned payload layout rather than a new invention.
Leaving this thread open until that lands. Thanks — this is the more valuable of the two findings; the layout bug I'd already caught, this one I had not.
Generated by Claude Code
| /// This IS the wire form: the value slab is these bytes, in this order. | ||
| #[must_use] | ||
| pub const fn as_payload_bytes(&self) -> &[u8; OPS_PER_FUNCTION] { | ||
| &self.ops |
There was a problem hiding this comment.
Emit the actual 480-byte facet wire layout
When a consumer uses this advertised wire form to populate a canonical node, it receives 360 contiguous opcode bytes, but the layout defined above is thirty repetitions of classid(4) + payload(12), totaling 480 bytes. Copying this value into the slab omits all 120 classid bytes and places every payload chunk after the first at the wrong offset, so a canonical loader cannot interpret the resulting row. This API needs to interleave the facet classids into a 480-byte representation, or be explicitly treated as logical payload with a separate canonical packer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and fixed in 0683ef3 — I hit the same defect independently while checking the density accounting, and landed the second of your two suggested resolutions: treat it as logical payload with a separate canonical packer.
as_payload_bytes→as_ops_bytes, documented as execution order and explicitly not the slab layoutslab_offset(i)=(i/12)*16 + 4 + i%12— the real mappingwrite_into_value_slab— the packer; scatters into the 12-byte payload lanes and touches no classid byteread_from_value_slab— the inverse gatherop_in_slab— a zero-copy lens reading one operation in place, never materialising the other 359
Named the constants the arithmetic was hiding (SLOT_STRIDE, CLASSID_BYTES, VALUE_SLAB_LEN) so the 480-vs-360 asymmetry is stated where someone hits it.
Three falsifiers pin it, each verified by breaking the scatter back into the naive contiguous copy: the interleave test asserts no operation sits at its own index (the mapping must be a genuine permutation) and that no offset lands on a classid byte; the round-trip test pre-stamps all 120 classid bytes with a sentinel and proves the write leaves every one intact; and a_naive_contiguous_copy_is_detectably_wrong fails if scatter and contiguous copy ever coincide, so this distinction cannot quietly become decorative.
Generated by Claude Code
… claim The first cut's as_payload_bytes doc said "the value slab is these bytes, in this order". That is false, and the defect it would have caused is concrete: a consumer doing slab[..360].copy_from_slice(body.as_payload_bytes()) shreds the first 22 and a half facets' classids and payloads alike. The value slab is 480 bytes of 30 x (classid 4 + payload 12), so operation i lives at stride 16, plus 4 into each facet -- slab_offset(i) = (i/12)*16 + 4 + i%12 -- and the 360 operation bytes are never a contiguous run. The gathered array and the slab layout are two different things and the API now says so. Renames as_payload_bytes to as_ops_bytes (it is execution order, not wire order) and adds the pieces that were missing: slab_offset, write_into_value_slab (scatter, touching only payload lanes), read_from_value_slab (gather, recovering len from the last non-NOP byte), and op_in_slab -- a zero-copy lens that reads one operation in place with a single indexed byte read instead of materialising the other 359. Also names the constants the arithmetic was hiding: SLOT_STRIDE, CLASSID_BYTES, VALUE_SLAB_LEN. The 480-vs-360 asymmetry is now documented where someone will hit it rather than inferred. Four new tests, all verified as real falsifiers by breaking the scatter into the naive contiguous copy: - the interleave test asserts no operation sits at its own index (the mapping must be a genuine permutation, not identity) and that no offset lands on a classid byte - the round-trip test pre-stamps all 120 classid bytes with a sentinel and proves the write leaves every one intact - a_naive_contiguous_copy_is_detectably_wrong fails if scatter and contiguous copy ever coincide, so the distinction this API draws cannot become decorative - the in-memory/wire test pins 362 vs 360 so the gap stays visible rather than surprising a consumer who assumed size_of equals payload size Adds examples/density.rs so the density figures are re-measurable rather than trusted: whole-node amortized cost is 512/360 = 1.422 bytes per operation at full occupancy, 2.844 at 180 ops, 5.689 at 90, 17.067 at 30. Ledger: docs/DISCOVERY-MAP.md D-BLOCKS-PALETTE (correction appended). Gates: 14 tests pass, fmt clean, clippy -D warnings clean, cargo check --workspace --all-targets clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DN5odWu4eisdevH9bPWyz
Codex P1 on #235 is correct: a body of bare palette bytes carries no operands. WAIT(1) and WAIT(10) produce identical bodies, and NUMBER says a literal is present without saying which one. The module docs claimed "one function = one node = up to 360 operations" as though that were a complete program representation. It is not. No fix for the gap here, because choosing the operand encoding is an architecture decision rather than an implementation detail. What this commit does is stop the crate advertising something it does not deliver, and name the two candidate resolutions so the decision is made deliberately: - immediates in the stream: some palette entries are followed by operand bytes (constant-pool index, variable slot), which keeps 360 as a byte bound, lowers the effective op count, and needs a home for the pool - (opcode : operand) pairs: read each 12-byte lane as the LE contract's L4 6 x (u8:u8) rail, giving 180 pairs per node -- operands become palette-addressed in the same byte space at the cost of halving the op budget, using a sanctioned payload layout rather than a new invention Until one lands, a FunctionBody is documented as an opcode skeleton and must not be advertised as a lossless program encoding. Gates: 14 tests pass, fmt clean, clippy -D warnings clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DN5odWu4eisdevH9bPWyz
Operator-ruled rework of the function body from a flat opcode stream to the V3 indexed reading: every 12-byte lane is 6x(u8:u8) rails indexed against a label codebook, and the unit is a Call -- function index : value byte(s). There is no opcode/function distinction any more. ADD is function 0x40, a user block is another index in the same <256 codebook, and invoking either is the same bytes. PaletteOp is renamed FnIndex; the Inventory SoA is the label codebook those indices resolve against, which is why labels stay out of payloads (slot purity) while one byte still names anything in scope. Three earlier defects/claims retire in place: - The "nesting gap" is withdrawn. It was an artifact of treating the body as self-delimiting bytecode. Nesting is by reference -- a function index names another function's node -- exactly as SB3 nests via block ids. No END marker, no jump offset, no need. - The operand gap (codex P1 on #235) closes. The value byte is the immediate (WAIT:10, REPEAT:4); computed arguments use a stack discipline ((NUMBER:5)(NUMBER:3)(ADD:_)); wide literals spend the value byte as a constant-pool index (pool is a named follow-up). - Arity is a classid property, not an encoding trick. LaneShape (mirroring CascadeShape G6D2/G4D3/G3D4) carves the same 360 bytes as 180 pairs / 120 triples / 90 quads. A function needing more immediates picks a wider carving, never a wider field. Narrowing is loud: BodyError::ValueBeyondShape refuses a call the shape would truncate instead of dropping a byte. Length recovery is call-level, per shape -- the byte-level rposition regression is caught by test in every shape. Both guards were verified by breaking them: the truncation guard fails the suite with "two immediates must not fit Pairs", the len regression with "Quads: trailing bare call lost". The retired edge-block design (12 in-family + 4 out-of-family) is stripped from this crate's docs; slot 1 is documented reserved- zeroed with the retirement named, so the deprecated shape cannot be re-learned from here. Relations ride the payload rails as indexed calls. Also recorded in the ledger: the operator's literal-over-grammar ruling (grammar lines like A = B + C are a projection over the pair stream, never the storage format) and the baby-steps roadmap -- ABI-shaped Blockly/Scratch first, later a PowerAutomate-shaped low-code editor, both Mario-editor ergonomics over ClassView : WideFieldMask projections. Ledger: docs/DISCOVERY-MAP.md D-BLOCKS-PALETTE (correction 2). Gates: 16 tests pass (2 falsifier break-runs verified), fmt clean, clippy -D warnings clean, workspace check + tests clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DN5odWu4eisdevH9bPWyz
Docs-only. Turns the session's rulings into a durable plan and records the a2ui-rs wiring as a ledger entry. docs/BLOCK-EDITOR-PLAN.md sequences W0-W5 with gates, five open decisions, and five falsifiers. It opens by naming what is LOCKED so a future session cannot re-derive it: the 0x17XX domain, the plug-and-play producer posture, the provenance fence, one content classid, (function:value) calls, arity-by-classid via LaneShape, nesting by reference, loud narrowing, literal-over-grammar, and the retired edge block. Every one is shipped code with tests (#234, #235, #236), not intention. The headline finding for the a2ui wiring is that most of it already exists. Charter C1.6 says a click IS a navigates_to/ActionInvocation edge, and a2ui-server already ships receive_action -> KlickwegEdge -> lower_action_fire -> ActionInvocation as pure compile-time value construction (#209, warden COMPILE-TIME-CLEAN, 34 tests). Placing a block, connecting two blocks and clicking a placed block are each a click by ordinal address, so each is already a Klickweg edge. Edit telemetry and harvested-app telemetry unify in one closed predicate set with NO new predicate -- the Predicate enum is count-locked at 79 and extending it is a gated ontology change, not a consequence of this arc. Nesting maps 1:1: the ObjectSlot "A3 Klickwege brick" recursion desktop -> window -> region -> widget becomes canvas -> script -> block -> input, which a2ui-wasm::resolve_nested walks unchanged. The gaps are recorded as measured, not assumed. Interaction-to-edge and nested addressing exist. A palette of pickables, 2-D placement (Skin::Form and Skin::Flow are both 1-D list renderers) and multi-facet body ingest (a2ui-wasm implements one 12-byte facet; a body is thirty) are absent -- none charter-forbidden, but the editor tier is a real build rather than wiring. Drag/connect is the one T2 pressure point: local drag state is fine, the result must travel as an address-carried write. Open decision D2 proposes that "place tile at slot N" rides ActionInvoke{ordinal: PLACE, args:[N, fn]} rather than a third FrameKind: args is explicitly ClassView/ActionDef-carved, so it is an address-carried write, and a third kind would widen a deliberately closed vocabulary. Roadmap order is operator-set: ABI-shaped Blockly/Scratch first, Klickwege wiring second, PowerAutomate-shaped skin third -- both skins Mario-editor ergonomics over ClassView : WideFieldMask, which is T1 applied at editor scale. The W1 falsifier is unchanged from the first turn of the arc: a drag produces zero SoA writes, an operand change exactly one. Ledger: docs/DISCOVERY-MAP.md D-BLOCKS-KLICKWEGE, graded [H] (PLAN) -- the W0 substrate it builds on is [G]/CODED, the wiring is unbuilt. Gates: docs-only; 16 ogar-blockly tests and workspace check re-verified unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DN5odWu4eisdevH9bPWyz
The plug-and-play producer crate for the
0x17XXBlocks domain reserved in #234 —ogar-obopattern, so concept ids are authoritative here and the shared CODEBOOK keeps zero0x17XXrows.Three collapses
Each retired a design question rather than answering it.
1 — The vocabulary is ONE BYTE. Commands and concepts share a 256-slot palette, so two frontends rendering the same operation land on the same slot:
logic_compare[LT]≡operator_lt≡PaletteOp::LT. That is where the Blockly↔Scratch convergence becomes real rather than nominal — same domain byte with different ids would only have looked converged.Measured from the two Apache-2.0 sources:
The dedup key is
(schema, code), notcode—RANDOMappears inmath,textandlistsas three different operations sharing a label.2 — Content needs ONE classid. Operations are payload bytes, not concept ids, so the per-operation concept space (and the 255-slot ceiling an earlier pass computed for it) does not need to exist.
BlockConcepthas two variants:ContentandInventory.3 — The body budget is derived, not chosen.
value(480)/16 = 30facet slots ×16−4 = 12payload bytes =OPS_PER_FUNCTION= 360, compile-asserted against the node layout. A longer function is split, never given a wider row — the canon's scale is the next cascade level, never field-widening, applied to program structure. It also makes "does this function fit?" checkable before writing instead of a surprise at write time.Storage — inventory SoA + N content SoAs, split by function
SoaSplitencodes the V3 mailbox doctrine rather than a storage preference: one function = one owner = its own SoA, so a registry read never touches a body and no shared table accumulates writers.Rejected alternative (considered this session): a Lance sidecar carrying a header whose schema defines the blob reading. Rejected because it reintroduces decode-before-address — the exact cost the key exists to avoid.
Palette ranges are prefix-routable
Shared computational core below
DEVICE_FAMILY_FLOOR = 0x90, sprite/stage families above it — so "is this operation frontend-specific?" is one compare, no table lookup. The device range is reserved, not allocated (108 measured; mint on demand). Slot0x00isNOP, the zero-fallback, so a partially-filled body needs no length field on the wire.Provenance fence
Enforced in the module docs: entries derive only from the Apache-2.0 Blockly and
scratch-blocksdefinitions, never from AGPLscratch-vm. That is what lets a GPL consumer link this public codebook while the GPL boundary stays inside the consumer repo.Falsifiers verified by breaking what they guard
Passing is not evidence a test can fail, so both load-bearing ones were checked by injection:
PROC_ARG→0x80) fails withPROC_ARG collides with VAR_GET at slot 0x80361 ops must not fitThe second run surfaced a real latent defect:
from_opsderivedlenfrom the caller's length rather than the copied count, so the guard alone keptops()in bounds and a future edit to it could have produced an out-of-rangelenand an out-of-bounds slice. Now derived from the write count — belt and braces.Gates
cargo test -p ogar-blockly— 10 pass, 0 failedcargo clippy -p ogar-blockly --all-targets -- -D warnings— cleancargo fmt -p ogar-blockly -- --check— cleancargo check --workspace --all-targets— exit 0cargo test --workspace— no failuresLedger
docs/DISCOVERY-MAP.md→D-BLOCKS-PALETTE(append-only,[G]CODED).Follow-up (not in this PR)
App-prefix allocation in
ports.rsfor the block consumers; the device-family palette range when a Scratch-style consumer needs it; the a2ui-rs seam (palette byte → tile, placement and clicks by ordinal address).Generated by Claude Code