Skip to content

Migrate bench-bsp-le-track2.rkt to current ATMS API (CI unblock)#10

Merged
hierophantos merged 1 commit into
mainfrom
claude/ci-fix-skip-bench-bsp-le-track2
Apr 25, 2026
Merged

Migrate bench-bsp-le-track2.rkt to current ATMS API (CI unblock)#10
hierophantos merged 1 commit into
mainfrom
claude/ci-fix-skip-bench-bsp-le-track2

Conversation

@kumavis
Copy link
Copy Markdown
Contributor

@kumavis kumavis commented Apr 25, 2026

Summary

raco pkg install --auto (the CI install step) was failing with:

benchmarks/micro/bench-bsp-le-track2.rkt:47:3: tms-write: unbound identifier

The file targeted the pre-D.5b TMS API (tms-write, tms-cell-value, tms-read, tms-commit) which was removed when the TMS was refactored into the tms-cell struct + atms-write-cell interface.

This PR migrates the benchmark to the current API rather than skipping it (the originally-considered approach in PR #2's commit f3a3ca3). The benchmark stays in the suite and continues to provide pre-Track-2 baseline data on the now-current ATMS API.

Migration shape

Old (deleted) New
tms-read tcv stack (depth-N walk over per-cell stack) atms-read-cell a key (walk values list until support ⊆ believed); depth N modeled by atms with N supported values + believed worldview = N-element assumption set
tms-write tcv stack value (push depth-N frame) atms-write-cell a key value support (O(1) struct-copy); cost dominated by support-set construction (hoisted out of the timed loop, mirroring the old benchmark's structure)
tms-commit tcv aid (promote contingent value to permanent) No direct analog. Replaced with atms-with-worldview benchmarks (alone, and switch + read) — the closest behavioural shape: changing the believed set so subsequent reads re-resolve

Other helpers (atms-empty, atms-assume, atms-amb, atms-add-nogood, atms-consistent?, set operations, make-prop-network, net-new-cell, net-add-propagator, run-to-quiescence) are unchanged — those names still exist on the post-refactor API.

Verification

  • raco make racket/prologos/benchmarks/micro/bench-bsp-le-track2.rkt succeeds on Racket 8.10 (CI uses 8.14)
  • Full bench runs end-to-end: 19 timed benchmarks reporting stable results
  • Sample from end-to-end run:
    • atms-read-cell depth=1 x100000 median 7.40 ms
    • atms-read-cell depth=10 x100000 median 57.34 ms — confirms the design intuition that read cost scales with depth
    • atms-write-cell depth=10 x50000 median 4.00 ms — write cost is roughly depth-independent, dominated by struct-copy

Why this approach instead of the skip

The earlier-considered fix on PR #2's commit f3a3ca3 adds compile-omit-paths to skip the file. That unblocks CI but loses a useful pre-track baseline. Migrating preserves the data the benchmark was intended to gather, on the API the system actually uses now.

Cross-PR impact

PRs #4, #5, #6, #7, #8, #9, #11, #12 all currently carry the compile-omit-paths skip as a CI-unblock commit. Once this PR lands, those PRs can drop their skip commit (it becomes redundant: the bench works on its own). Strictly merge this first, then rebase the others to drop the duplicate.

Commits

  1. 3e03516 — migrate bench file (97 insertions, 33 deletions)

https://claude.ai/code/session_01MbncYJnrvjzhbVWw4xGi5x

The file targeted the pre-D.5b TMS API (`tms-write`, `tms-cell-value`,
`tms-read`, `tms-commit`) which was removed when the TMS was refactored
into the `tms-cell` struct + `atms-write-cell` interface. `raco pkg
install --auto` was failing on `unbound identifier: tms-write`.

Migration shape:

* TMS read at depth N → atms-read-cell against an atms whose target cell
  carries N supported values, with the believed worldview = N-element
  assumption set. Read cost is O(values-length) walking the values list
  + a hash-subset? against an N-element support, mirroring the old
  O(stack-depth) walk.
* TMS write at depth N → atms-write-cell with a pre-built N-element
  support hasheq. The atms-write-cell call itself is O(1); the meaningful
  cost is constructing the support set, which the benchmark does once
  outside the timed loop (matching the old benchmark's structure where
  the stack was hoisted).
* TMS commit → no direct analog post-refactor. Replaced with worldview
  switching benchmarks (atms-with-worldview alone, and switch + read)
  which are the closest behavioural shape: changing the believed set so
  subsequent reads re-resolve.

Other helpers (atms-empty / atms-assume / atms-amb / atms-add-nogood /
atms-consistent? / set operations / make-prop-network / net-new-cell /
net-add-propagator / run-to-quiescence) are unchanged — those names
still exist on the post-refactor API.

Verified locally on Racket 8.10 (CI uses 8.14): the file now compiles
via `raco make` and the full bench runs end-to-end with 19 timed
benchmarks reporting stable results.

This replaces the earlier-considered `compile-omit-paths` skip
(pre-existing on PR #2's branch f3a3ca3) with a real fix, so the
benchmark stays in the suite and continues to provide pre-Track-2
baseline data on the (now current) ATMS API.

https://claude.ai/code/session_01MbncYJnrvjzhbVWw4xGi5x

Co-authored-by: kumavis <1474978+kumavis@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@hierophantos hierophantos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the careful migration. Three things I appreciated: the honest "no direct analog" note for tms-commit (forcing an analog would have produced misleading numbers), the string->symbol correctness fix that the old benchmark was getting away with by accident, and the merge-this-first sequencing call in the PR body — saved us a rebase round. Approving and merging.

@hierophantos hierophantos merged commit bd3f8f8 into main Apr 25, 2026
1 check passed
@hierophantos hierophantos deleted the claude/ci-fix-skip-bench-bsp-le-track2 branch April 25, 2026 23:25
kumavis added a commit that referenced this pull request Apr 26, 2026
The 2026-04-23 eigentrust pitfalls memo (forthcoming branch) enumerated 16
items hit during the EigenTrust implementation. Items #1-7 and #11-15 are
language/elaboration defects with their own PRs. Items #8, #9, #10, and #16
are observations rather than Prologos defects; no compiler change is needed
for them but the memo deserves a parallel disposition note so a future reader
does not double-count them as open work.

#8 (exact-Rat slow on deep iter): intrinsic to exact rational arithmetic;
benchmark-scope guidance, not a fix.

#9 (Posit32 literals work): positive observation; `~` literal prefix is
unambiguous unlike `0/1`. No action.

#10 (PVec preserves where List does not): subsumed by pitfall #3 fix. After
#3 lands, both literal forms preserve element type uniformly. Close as
duplicate.

#16 (column-stochastic vs row-stochastic): algorithm/spec clarification, not
a Prologos defect. The eigentrust implementation branch already takes
column-stochastic M directly and validates via col-stochastic?.

https://claude.ai/code/session_01MbncYJnrvjzhbVWw4xGi5x

Co-authored-by: kumavis <1474978+kumavis@users.noreply.github.com>
kumavis added a commit that referenced this pull request Apr 26, 2026
The 2026-04-23 eigentrust pitfalls memo (forthcoming branch) enumerated 16
items hit during the EigenTrust implementation. Items #1-7 and #11-15 are
language/elaboration defects with their own PRs. Items #8, #9, #10, and #16
are observations rather than Prologos defects; no compiler change is needed
for them but the memo deserves a parallel disposition note so a future reader
does not double-count them as open work.

#8 (exact-Rat slow on deep iter): intrinsic to exact rational arithmetic;
benchmark-scope guidance, not a fix.

#9 (Posit32 literals work): positive observation; `~` literal prefix is
unambiguous unlike `0/1`. No action.

#10 (PVec preserves where List does not): subsumed by pitfall #3 fix. After
#3 lands, both literal forms preserve element type uniformly. Close as
duplicate.

#16 (column-stochastic vs row-stochastic): algorithm/spec clarification, not
a Prologos defect. The eigentrust implementation branch already takes
column-stochastic M directly and validates via col-stochastic?.

https://claude.ai/code/session_01MbncYJnrvjzhbVWw4xGi5x

Co-authored-by: kumavis <1474978+kumavis@users.noreply.github.com>
kumavis pushed a commit that referenced this pull request Apr 27, 2026
Per user review of #0-#10: many entries were either out-of-scope
(env limitations, not Prologos issues) or wrong (claims I never
actually tested). Re-tested every claim against a real Racket and
revised the doc.

Numbers are reserved per the user's instruction — entries marked
DELETED keep their slot so cross-refs don't drift.

Detail:

  #0  DELETED — out-of-scope (Racket toolchain not in sandbox).
                Environment limitation, not a Prologos issue.

  #1  REFRAMED — was "capability subtype + promise resolution
                composition." Re-titled to honestly reflect what
                this actually is: an OCapN-side Phase 0
                deferred-implementation note (eventual cross-vat
                receive isn't wired up yet). NOT a Prologos bug.

  #2  DELETED — false claim. Tested with a real Racket: WS-mode
                wildcard match `match | _ -> body` on user data
                types elaborates AND evaluates correctly when the
                function carries a proper `spec`. The
                `prologos::data::datum` comment I cited applies to
                a narrower polymorphic-context case, not a blanket
                wildcard ban as I asserted.
                Cleanup of behavior.prologos (~250 -> ~70 LOC)
                follows.

  #3  DELETED — false claim. Tested: `data Step step : [Nat -> Nat]`
                (with bracketed function type per the lseq-cell
                convention) accepts a function value, including
                closures with captured state. Open-world actor
                behaviour storage IS supported. The closed-enum
                BehaviorTag in our implementation was a needless
                workaround driven by this incorrect pitfall.
                Cleanup tracked separately.

  #4  KEPT, REFRAMED — real, narrowed claim. grammar.ebnf §6
                lines 1153/1187/1199 promise `Mu` (sexp) and `rec`
                (WS) for recursive sessions. Both elaborate to
                `Unknown session type: rec` / `Mu`. So pitfall #4
                is now: "rec/Mu in grammar but not in elaborator."
                CapTP's stream-level well-typedness is therefore
                the documented ceiling; per-exchange sub-protocols
                remain the workaround.

  #5  KEPT — `none`/`some` need explicit type args in some inference
            contexts. Real ergonomics tension, accurately
            documented.

  #6  DELETED — out-of-scope. WS-mode `let p := body` and sexp-mode
                `(let (p v) body)` are TWO surface forms by design
                (grammar.ebnf §7 line 1236). User-error, not a
                Prologos bug.

  #7  DELETED — was a quantitative restatement of #2. With #2
                recanted, #7 evaporates: behavior modules can be
                wildcard-collapsed, dropping ~180 LOC.

  #8  DELETED — false claim. Tested: `data Box1 box1 : [Sigma [_ <Nat>] Bool]`
                and `data Table table : Nat -> [List [Sigma [_ <Nat>] Bool]]`
                both elaborate cleanly. The named-struct
                ActorEntry/PromiseEntry workaround in vat.prologos
                was unnecessary; can be simplified back.

  #9  DELETED — user error. `def` for value bindings vs `defn` for
                functions is documented (grammar.ebnf §3
                lines 189-190, prologos-syntax rules). Mis-using
                `defn` for a 0-ary constant isn't a Prologos bug.

  #10 DELETED — out-of-scope. Network sandbox blocking external
                docs is an environment limitation.

#11-#20 were not in scope of this review and remain as-is for the
user to review next.
kumavis pushed a commit that referenced this pull request May 4, 2026
Per user review of #0-#10: many entries were either out-of-scope
(env limitations, not Prologos issues) or wrong (claims I never
actually tested). Re-tested every claim against a real Racket and
revised the doc.

Numbers are reserved per the user's instruction — entries marked
DELETED keep their slot so cross-refs don't drift.

Detail:

  #0  DELETED — out-of-scope (Racket toolchain not in sandbox).
                Environment limitation, not a Prologos issue.

  #1  REFRAMED — was "capability subtype + promise resolution
                composition." Re-titled to honestly reflect what
                this actually is: an OCapN-side Phase 0
                deferred-implementation note (eventual cross-vat
                receive isn't wired up yet). NOT a Prologos bug.

  #2  DELETED — false claim. Tested with a real Racket: WS-mode
                wildcard match `match | _ -> body` on user data
                types elaborates AND evaluates correctly when the
                function carries a proper `spec`. The
                `prologos::data::datum` comment I cited applies to
                a narrower polymorphic-context case, not a blanket
                wildcard ban as I asserted.
                Cleanup of behavior.prologos (~250 -> ~70 LOC)
                follows.

  #3  DELETED — false claim. Tested: `data Step step : [Nat -> Nat]`
                (with bracketed function type per the lseq-cell
                convention) accepts a function value, including
                closures with captured state. Open-world actor
                behaviour storage IS supported. The closed-enum
                BehaviorTag in our implementation was a needless
                workaround driven by this incorrect pitfall.
                Cleanup tracked separately.

  #4  KEPT, REFRAMED — real, narrowed claim. grammar.ebnf §6
                lines 1153/1187/1199 promise `Mu` (sexp) and `rec`
                (WS) for recursive sessions. Both elaborate to
                `Unknown session type: rec` / `Mu`. So pitfall #4
                is now: "rec/Mu in grammar but not in elaborator."
                CapTP's stream-level well-typedness is therefore
                the documented ceiling; per-exchange sub-protocols
                remain the workaround.

  #5  KEPT — `none`/`some` need explicit type args in some inference
            contexts. Real ergonomics tension, accurately
            documented.

  #6  DELETED — out-of-scope. WS-mode `let p := body` and sexp-mode
                `(let (p v) body)` are TWO surface forms by design
                (grammar.ebnf §7 line 1236). User-error, not a
                Prologos bug.

  #7  DELETED — was a quantitative restatement of #2. With #2
                recanted, #7 evaporates: behavior modules can be
                wildcard-collapsed, dropping ~180 LOC.

  #8  DELETED — false claim. Tested: `data Box1 box1 : [Sigma [_ <Nat>] Bool]`
                and `data Table table : Nat -> [List [Sigma [_ <Nat>] Bool]]`
                both elaborate cleanly. The named-struct
                ActorEntry/PromiseEntry workaround in vat.prologos
                was unnecessary; can be simplified back.

  #9  DELETED — user error. `def` for value bindings vs `defn` for
                functions is documented (grammar.ebnf §3
                lines 189-190, prologos-syntax rules). Mis-using
                `defn` for a 0-ary constant isn't a Prologos bug.

  #10 DELETED — out-of-scope. Network sandbox blocking external
                docs is an environment limitation.

#11-#20 were not in scope of this review and remain as-is for the
user to review next.
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.

3 participants