hero_rpc#122: codegen fixes + osis benches + ClientT impl #125

Merged

timur merged 1 commit from issue-122-template-revalidation into development 2026-05-22 07:42:07 +00:00

Conversation 0 Commits 1 Files changed 35 +2905 -112

timur commented 2026-05-22 07:41:17 +00:00

Owner

Copy link

Summary

Closes #122 (Phases 1, 3, 4 — Phase 2 lives in
lhumina_code/hero_service#issue-122-template-revalidation).

Codegen / template fixes surfaced by re-validating the canonical
hero_service template against the current generator, plus the
criterion benchmark harness this issue exists to produce.

What changed

Codegen / scaffolder fixes

File	Fix
generator/build/scaffold.rs	Drop .with_wasm() from emitted build.rs (config method removed in #114).
generator/build/scaffold.rs	SDK Cargo.toml declares hero_rpc2 + jsonrpsee (proc-macro expansion needs both) and a build-dep edge on the core crate so cargo waits for codegen before compiling the SDK.
generator/build/scaffold.rs	SDK lib.rs stub now exposes pub mod generated; pub use generated::*;.
generator/build/scaffold.rs	tests/src/lib.rs uses concrete Arc<Client> (jsonrpsee 0.26's ClientT is not dyn-compatible) and a ServiceHandle::shutdown() method that the emitted e2e tests call.
generator/build/ui_emit.rs	Admin/web create route uses _new(input) -> sid (the #117 shape), not _set(obj). FieldKind::Json form fields parse back via serde_json::from_str instead of assigning String to Vec<…>.
generator/generate/rust_server.rs	When nested_layout pushes the core import one super deeper, the handler use super::super::super::* line now finds the right anchor instead of getting injected above the module doc comment.
generator/rust/rust_struct.rs	Drop the now-unused OtomlSerialize import from the emitted types.rs — the pub use … as _ re-export covers it.
generator/rust/rust_osis.rs	OsisAppRpcHandler::type_names() and the handle_rpc match arms use snake_case (indexed_multi.new), matching OsisObject::type_name() and the SDK trait #[method(name = "…")] annotations. Multi-word rootobjects were failing with MethodNotFound on the wire path.
hero_rpc2/src/client.rs	impl ClientT for hero_rpc2::Client so the generated SDK trait methods extend straight off Arc<Client> — the existing Deref to InnerClient didn't help trait-method dispatch.

Benchmark harness

New crates/osis_benches/ — criterion harness covering five cases:
set_throughput, get_by_sid_latency,
query_indexed_vs_full_scan (headline), query_multi_index,
index_build_from_empty. Spins up the OSIS server via
hero_rpc_osis::rpc::bootstrap::run_for_test so it exercises the
same path production binaries take.

BENCH_RESULTS.md (committed at the repo root) captures real numbers
from a local run on an M1 MacBook Pro.

Headline number

query_indexed_vs_full_scan at 5 000 rows, same IndexedSingle
dataset on both arms, probe word "Aurora":

Arm	Mean	Read
shadow_indexed.title (HashMap built from indexed_fields())	1.349 ms	what @index would buy once OSIS consults it
full_scan.title (list_full() + filter())	121.98 ms	the only query shape the wire path actually supports today

~90× gap. That's the budget a real indexed_fields-consuming
storage backend has to play in. The current OSIS code path never
calls indexed_fields() and the SDK trait emits no _find_*
method — so the entire 90× is on the table waiting to be claimed
by the follow-up issue.

Acceptance criteria

From #122:

• cargo build --workspace clean, zero warnings. (Verified locally; one pre-existing unused-import in crates/osis/examples/basic/code/ was already broken on development and is not from this branch.)
• cargo test --workspace --lib --bins clean — 318+ unit tests pass.
• Catalog schema extended with IndexedNone / IndexedSingle / IndexedMulti / IndexedNonStr (in hero_service — see the paired PR).
• OsisObject::indexed_field_names() correct on all four rootobjects.
• BENCH_RESULTS.md committed; query_indexed_vs_full_scan measurable gap.
• No co-author trailers (per feedback_commit_attribution.md).

Decisions taken without confirmation

• Bench fixture lives in a dedicated crates/osis_benches/ crate rather than crates/osis/benches/ so the codegen pipeline runs through build.rs (matching consumer service shape). Pro: every generated service inherits the pattern. Con: an extra workspace member.
• @index is metadata-only — benchmark shows a shadow-index ceiling, not a wire-path number. Per #122's "real numbers, not theatrical ones" rule, this is the honest finding. The follow-up issue #123 is the actual fix.
• MultiDomainBuilder extraction deferred to the alignment follow-up #124, so the workspace-root tests/src/lib.rs template still hand-rolls its registration ladder for two domains.

Follow-ups not done here

• #123 — OSIS @index integration: wire find_* through hero_indexer with typed <RootObject>FindParams. The 90× gap above is the motivation.
• #124 — Service lifecycle alignment: collapse the in-process spin_up_service duplication into a MultiDomainBuilder + ship the nushell tests/smoke.nu / tests/api_integration.nu Layer-2/3 scripts the tests_pyramid skill describes.
• wasm32 hero_rpc2 transitive tokio/mio gap — status unchanged from #119; not in scope here.

Test plan

• cargo build -p hero_rpc_generator clean.
• cargo test --workspace --lib --bins clean.
• cargo bench -p hero_rpc_osis_benches --bench index_perf -- --quick produces criterion output and the headline gap above.
• hero_service e2e tests pass via the paired PR.

🤖 Generated with Claude Code

## Summary Closes #122 (Phases 1, 3, 4 — Phase 2 lives in [lhumina_code/hero_service#issue-122-template-revalidation](https://forge.ourworld.tf/lhumina_code/hero_service/pulls)). Codegen / template fixes surfaced by re-validating the canonical `hero_service` template against the current generator, plus the criterion benchmark harness this issue exists to produce. ## What changed ### Codegen / scaffolder fixes | File | Fix | |------|-----| | `generator/build/scaffold.rs` | Drop `.with_wasm()` from emitted `build.rs` (config method removed in #114). | | `generator/build/scaffold.rs` | SDK Cargo.toml declares `hero_rpc2` + `jsonrpsee` (proc-macro expansion needs both) and a build-dep edge on the core crate so cargo waits for codegen before compiling the SDK. | | `generator/build/scaffold.rs` | SDK `lib.rs` stub now exposes `pub mod generated; pub use generated::*;`. | | `generator/build/scaffold.rs` | `tests/src/lib.rs` uses concrete `Arc<Client>` (jsonrpsee 0.26's `ClientT` is not dyn-compatible) and a `ServiceHandle::shutdown()` method that the emitted e2e tests call. | | `generator/build/ui_emit.rs` | Admin/web `create` route uses `_new(input) -> sid` (the #117 shape), not `_set(obj)`. `FieldKind::Json` form fields parse back via `serde_json::from_str` instead of assigning `String` to `Vec<…>`. | | `generator/generate/rust_server.rs` | When `nested_layout` pushes the core import one super deeper, the handler `use super::super::super::*` line now finds the right anchor instead of getting injected above the module doc comment. | | `generator/rust/rust_struct.rs` | Drop the now-unused `OtomlSerialize` import from the emitted `types.rs` — the `pub use … as _` re-export covers it. | | `generator/rust/rust_osis.rs` | `OsisAppRpcHandler::type_names()` and the `handle_rpc` match arms use snake_case (`indexed_multi.new`), matching `OsisObject::type_name()` and the SDK trait `#[method(name = "…")]` annotations. Multi-word rootobjects were failing with `MethodNotFound` on the wire path. | | `hero_rpc2/src/client.rs` | `impl ClientT for hero_rpc2::Client` so the generated SDK trait methods extend straight off `Arc<Client>` — the existing `Deref` to `InnerClient` didn't help trait-method dispatch. | ### Benchmark harness New `crates/osis_benches/` — criterion harness covering five cases: `set_throughput`, `get_by_sid_latency`, `query_indexed_vs_full_scan` (headline), `query_multi_index`, `index_build_from_empty`. Spins up the OSIS server via `hero_rpc_osis::rpc::bootstrap::run_for_test` so it exercises the same path production binaries take. `BENCH_RESULTS.md` (committed at the repo root) captures real numbers from a local run on an M1 MacBook Pro. ## Headline number `query_indexed_vs_full_scan` at 5 000 rows, same `IndexedSingle` dataset on both arms, probe word `"Aurora"`: | Arm | Mean | Read | |-----|-----:|------| | `shadow_indexed.title` (HashMap built from `indexed_fields()`) | **1.349 ms** | what `@index` *would* buy once OSIS consults it | | `full_scan.title` (`list_full() + filter()`) | **121.98 ms** | the only query shape the wire path actually supports today | **~90× gap.** That's the budget a real `indexed_fields`-consuming storage backend has to play in. The current OSIS code path never calls `indexed_fields()` and the SDK trait emits no `_find_*` method — so the entire 90× is on the table waiting to be claimed by the follow-up issue. ## Acceptance criteria From #122: - [x] `cargo build --workspace` clean, zero warnings. (Verified locally; one pre-existing unused-import in `crates/osis/examples/basic/code/` was already broken on `development` and is not from this branch.) - [x] `cargo test --workspace --lib --bins` clean — 318+ unit tests pass. - [x] Catalog schema extended with `IndexedNone` / `IndexedSingle` / `IndexedMulti` / `IndexedNonStr` (in `hero_service` — see the paired PR). - [x] `OsisObject::indexed_field_names()` correct on all four rootobjects. - [x] `BENCH_RESULTS.md` committed; `query_indexed_vs_full_scan` measurable gap. - [x] No co-author trailers (per `feedback_commit_attribution.md`). ## Decisions taken without confirmation - **Bench fixture lives in a dedicated `crates/osis_benches/` crate** rather than `crates/osis/benches/` so the codegen pipeline runs through `build.rs` (matching consumer service shape). Pro: every generated service inherits the pattern. Con: an extra workspace member. - **`@index` is metadata-only — benchmark shows a shadow-index ceiling, not a wire-path number.** Per #122's "real numbers, not theatrical ones" rule, this is the honest finding. The follow-up issue #123 is the actual fix. - **`MultiDomainBuilder` extraction deferred** to the alignment follow-up #124, so the workspace-root `tests/src/lib.rs` template still hand-rolls its registration ladder for two domains. ## Follow-ups not done here - **#123** — OSIS `@index` integration: wire `find_*` through `hero_indexer` with typed `<RootObject>FindParams`. The 90× gap above is the motivation. - **#124** — Service lifecycle alignment: collapse the in-process `spin_up_service` duplication into a `MultiDomainBuilder` + ship the nushell `tests/smoke.nu` / `tests/api_integration.nu` Layer-2/3 scripts the `tests_pyramid` skill describes. - **wasm32 hero_rpc2 transitive tokio/mio gap** — status unchanged from #119; not in scope here. ## Test plan - [x] `cargo build -p hero_rpc_generator` clean. - [x] `cargo test --workspace --lib --bins` clean. - [x] `cargo bench -p hero_rpc_osis_benches --bench index_perf -- --quick` produces criterion output and the headline gap above. - [x] hero_service e2e tests pass via the paired PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

timur added 1 commit 2026-05-22 07:41:17 +00:00

hero_rpc#122: codegen fixes + osis benches + ClientT impl ... e4b35658d8

Closes hero_rpc#122 Phases 1, 3, 4. Phase 2 lives in lhumina_code/hero_service.

Codegen / template fixes surfaced by re-validating the hero_service
template against the current generator:

- scaffold.rs: drop `.with_wasm()` from the emitted build.rs (config
  method gone since #114).
- scaffold.rs: SDK Cargo.toml now declares `hero_rpc2` + `jsonrpsee`
  (proc-macro expansion needs both) and a build-dep edge on the core
  crate so cargo waits for codegen before compiling the SDK.
- scaffold.rs: SDK lib.rs stub now `pub mod generated; pub use
  generated::*;` so consumers can resolve `<sdk>::<domain>::<Trait>Client`.
- scaffold.rs: tests/src/lib.rs uses the concrete `Arc<Client>`
  (jsonrpsee 0.26's `ClientT` is not dyn-compatible) and exposes a
  `ServiceHandle::shutdown()` method the emitted e2e tests call.
- ui_emit.rs: admin/web `create` route uses `_new(input) -> sid` (the
  #117 shape), not `_set(obj)`. `FieldKind::Json` form fields parse
  back via `serde_json::from_str` instead of assigning a `String` to
  a `Vec<…>`.
- generate/rust_server.rs: when nested_layout pushes the core import
  one super deeper, the handler `use super::super::super::*` line
  finds the right anchor instead of getting injected above the module
  doc comment.
- rust_struct.rs: drop the now-unused `OtomlSerialize` import from the
  emitted types.rs — the `pub use … as _` re-export covers it.
- rust_osis.rs: `OsisAppRpcHandler::type_names()` and the
  `handle_rpc` match-arm keys use snake_case (`indexed_multi.new`),
  matching `OsisObject::type_name()` and the SDK trait
  `#[method(name = "…")]` annotations. Multi-word rootobjects were
  failing with `MethodNotFound` on the wire path.

Runtime:

- hero_rpc2: `impl ClientT for hero_rpc2::Client` so the generated
  SDK trait methods extend straight off `Arc<Client>` — the existing
  Deref to `InnerClient` didn't help trait-method dispatch.

Benches:

- new `crates/osis_benches/`: criterion harness with five cases — set
  throughput, get_by_sid_latency, query_indexed_vs_full_scan (headline),
  query_multi_index, index_build_from_empty. Uses
  `hero_rpc_osis::rpc::bootstrap::run_for_test` so the OSIS server
  comes up the same way production binaries do.
- BENCH_RESULTS.md: real numbers from a local run. Headline: at 5k
  rows the indexed arm (shadow HashMap built from `indexed_fields()`)
  is ~1.35 ms; the only wire-path query shape today
  (`list_full + filter()`) is ~122 ms — a ~90× gap left on the table
  because `@index` is metadata-only in OSIS today.
- The finding above is the motivation for follow-up issue #123 (wire
  `find_*` through hero_indexer with typed FindParams).
- Follow-up #124 covers collapsing the in-process `spin_up_service`
  duplication into a multi-domain bootstrap builder + the missing
  Layer 2–4 nushell scripts per the `tests_pyramid` skill.

.gitignore: hide the workspace-root `docs/openrpc.json` /
`docs/bench/` aggregates that `osis_benches` build.rs emits.

timur referenced this pull request from lhumina_code/hero_service 2026-05-22 07:41:54 +00:00

hero_rpc#122 Phase 2: regen template against current hero_rpc + bench schema #7

timur merged commit b4b7623149 into development 2026-05-22 07:42:07 +00:00

timur referenced this pull request from a commit 2026-05-22 07:42:09 +00:00

hero_rpc#122: codegen fixes + osis benches + ClientT impl (#125)

timur referenced this pull request 2026-05-22 07:43:43 +00:00

Re-validate hero_service template + benchmark osis with real indexes #122

timur referenced this pull request 2026-05-22 09:21:27 +00:00

hero_rpc#124: MultiDomainBuilder + subprocess-driver tests + Layer 2-4 nu skeletons #126

timur referenced this pull request from lhumina_code/hero_skills 2026-05-22 10:09:13 +00:00

[META] Standardize Hero service architecture — template, hero_rpc alignment, framework hoist, skills #262

timur referenced this pull request 2026-05-22 10:09:14 +00:00

Generator + scaffolder: emit per-root-object Criterion benchmarks from OSchema #113

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

prio_critical

  

prio_low

  

type_bug

  

type_contact

  

type_issue

  

type_lead

  

type_question

  

type_story

  

type_task

No labels

prio_critical

prio_low

type_bug

type_contact

type_issue

type_lead

type_question

type_story

type_task

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

lhumina_code/hero_rpc!125

No description provided.