Stop regenerating herolib_core::base ServiceToml types per-service #72

New issue

Closed

opened 2026-05-19 16:47:00 +00:00 by timur · 3 comments

timur commented 2026-05-19 16:47:00 +00:00

Owner

Copy link

Problem

crates/<name>/src/<domain>/types_generated.rs includes generated Rust types for Kind, Category, Protocol, SockType, Binary, Socket, ServiceMeta, etc. — i.e. the ServiceToml schema that already lives in herolib_core::base.

Example: hero_service/crates/hero_service/src/catalog/types_generated.rs.

This is duplicated work + a maintenance trap: the canonical types live in herolib_core and lab parses service.toml against those types directly. No service code should ever need re-emitted copies.

Why is it happening?

Likely the scaffolder's seed schema (services.oschema) for the hero_service toy domain mirrors the ServiceToml shape, so the generator dutifully emits Rust types for it. Two fixes apply:

• (Specific) The hero_service template's services.oschema should not redeclare types that already exist in herolib_core::base. It should reference / import them.
• (General) The generator should support importing external types in an OSchema (or annotating types as "external") so they aren't re-emitted.

What to do

• Add an OSchema construct to declare "this type is provided by crate herolib_core::base" so the generator emits an import, not a redefinition.
• Update hero_service's services.oschema to use it for the ServiceToml types.
• Verify hero_service rebuilds with the canonical types in scope and zero duplicate definitions.

Acceptance

• hero_service/crates/hero_service/src/.../types_generated.rs no longer contains Kind/Category/Protocol/SockType/Binary/Socket/ServiceMeta redefinitions.
• Generated code imports from herolib_core::base instead.
• All consumers compile clean.

## Problem `crates/<name>/src/<domain>/types_generated.rs` includes generated Rust types for `Kind`, `Category`, `Protocol`, `SockType`, `Binary`, `Socket`, `ServiceMeta`, etc. — i.e. the [ServiceToml schema](https://forge.ourworld.tf/lhumina_code/hero_lib/src/branch/development/crates/core/src/base/service.rs) that already lives in `herolib_core::base`. Example: [`hero_service/crates/hero_service/src/catalog/types_generated.rs`](https://forge.ourworld.tf/lhumina_code/hero_service/src/branch/development/crates/hero_service/src/catalog/types_generated.rs). This is duplicated work + a maintenance trap: the canonical types live in `herolib_core` and `lab` parses `service.toml` against those types directly. No service code should ever need re-emitted copies. ## Why is it happening? Likely the scaffolder's seed schema (`services.oschema`) for the `hero_service` toy domain mirrors the ServiceToml shape, so the generator dutifully emits Rust types for it. Two fixes apply: - (Specific) The `hero_service` template's `services.oschema` should not redeclare types that already exist in `herolib_core::base`. It should reference / import them. - (General) The generator should support importing external types in an OSchema (or annotating types as "external") so they aren't re-emitted. ## What to do - Add an OSchema construct to declare "this type is provided by crate `herolib_core::base`" so the generator emits an import, not a redefinition. - Update `hero_service`'s `services.oschema` to use it for the ServiceToml types. - Verify `hero_service` rebuilds with the canonical types in scope and zero duplicate definitions. ## Acceptance - `hero_service/crates/hero_service/src/.../types_generated.rs` no longer contains `Kind`/`Category`/`Protocol`/`SockType`/`Binary`/`Socket`/`ServiceMeta` redefinitions. - Generated code imports from `herolib_core::base` instead. - All consumers compile clean.

timur referenced this issue from lhumina_code/hero_skills 2026-05-20 00:15:53 +00:00

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

timur referenced this issue from lhumina_code/hero_skills 2026-05-20 05:57:18 +00:00

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

timur referenced this issue from lhumina_code/hero_skills 2026-05-20 06:53:09 +00:00

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

timur referenced this issue from lhumina_code/hero_skills 2026-05-20 06:59:12 +00:00

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

timur commented 2026-05-20 07:16:45 +00:00

Author

Owner

Copy link

Design proposal — OSchema external types

Picking up #72. Before coding, I'd like to lock the OSchema syntax for declaring "this type lives in an external Rust crate, don't redefine it." Posting here for sign-off.

Context — what the failure case actually looks like

The issue points at hero_service. The on-disk file is crates/hero_service/schemas/catalog/catalog.oschema (not services.oschema — services.oschema got renamed to catalog.oschema after the refactor that introduced ServiceDefinition).

Today that file redeclares what herolib_core::base already owns:

OSchema redeclaration	Canonical (herolib_core::base)
BinaryKind	Kind
SocketKind	SockType
SocketProtocol	Protocol
ServiceCategory	Category
SocketDef	Socket
BinaryDef	Binary

The closed-set enums even drift (e.g. BinaryKind has App, Kind doesn't; ServiceCategory is tool/framework/infrastructure/example vs Category's core/storage/ai/network/ui/tool). That drift is exactly the maintenance trap #72 calls out.

Proposed syntax — use statements

I propose a top-level use statement that mirrors Rust's use exactly. It's familiar, unambiguous, and trivially translates to the Rust generator's output:

use herolib_core::base::{Kind, Category, Protocol, SockType, Binary, Socket, ServiceMeta, ServiceToml}

# Now `Kind`, `Binary`, etc. refer to the imported types — never redefined locally.
# Catalog-owned types still live in this file.
ServiceDefinition = {
    sid:        str               # SmartID identifier (assigned by OSIS)
    name:       str @index
    repo:       str @index
    meta:       ServiceMeta       # ← resolves to herolib_core::base::ServiceMeta
    binaries:   [Binary]          # ← resolves to herolib_core::base::Binary
    schema_files: [str]
    tags:       [str]
    created_at: otime
    updated_at: otime
}

Single-import form (for the one-type case) also accepted:

use herolib_core::base::ServiceToml

Why use and not an attribute on the existing redefinition?

I considered an alternative — keep the redeclaration but tag it @external("herolib_core::base::Binary"):

@external("herolib_core::base::Binary")
Binary = { name: str, kind: BinaryKind, ... }

I'm proposing against it because the redeclaration is the bug — the whole point of #72 is that the local shape is a copy that can drift. Letting authors keep writing a shape next to an @external annotation invites that drift back in. use makes the name visible without ever writing a shape, which is exactly what we want.

Semantics — what the model + generators do

Parser / model:

• A new top-level statement: use <crate>::<path>::{Ident, Ident, ...} or use <crate>::<path>::Ident.
• Stored on Schema as a Vec<ExternalImport> (each carries the crate path + imported names).
• The name resolver treats an imported name as a valid target for TypeExprKind::Named(name) — references compile through with no local definition required.
• Duplicate import + local-definition for the same name = parse error ("name X is both imported and defined locally").

Rust generator (types_generated.rs):

• For each use crate::path::{A, B} → emit use crate::path::{A, B}; at the top of the file.
• The generator's existing per-type emission loop iterates schema.definitions — imports are not in there, so imported names are never redefined.

Rust WASM generator (types_wasm_generated.rs):

• This is the tricky one — herolib_core may not be wasm-friendly. Two reasonable choices:
  1. Emit the same use and let cargo sort it out (simpler — if the external crate isn't wasm-compatible, the user finds out at compile time, which is honest).
  2. Per-target imports, e.g. use crate::path::{A, B} for wasm = "..." — adds syntax for an edge case nobody's hit yet.
• I'd ship (1) and only add per-target overrides when someone actually needs it.

OpenRPC generator:

• Imported names emit as { "$ref": "#/components/schemas/Name" } references like any other named type.
• The schema for Name is emitted as { "type": "object", "description": "External type from <crate path>" } — opaque, but valid JSON Schema. Generators of typed clients (TS, Python) get a black-box type they can pass through. If we ever need a richer description, the external crate can ship its own JSON Schema alongside.

Rhai / JS / Python generators:

• Skip emission for imported names with a comment "external type, not generated here." Anyone who needs a Rhai binding for Binary will get one from the upstream crate.

Acceptance, restated against this design

After the change, hero_service/crates/hero_service/src/catalog/types_generated.rs will:

• Contain use herolib_core::base::{Kind, Category, Protocol, SockType, Binary, Socket, ServiceMeta, ServiceToml}; near the top.
• Not contain any pub enum Kind, pub struct Binary, etc.
• Still contain ServiceDefinition (the one type hero_service legitimately owns).
• cargo build -p hero_service clean — no duplicates, references resolve through the import.

Open questions for sign-off

1. use keyword OK? It collides with nothing in OSchema today (use is not a reserved identifier). If you'd rather a less Rust-flavored word (import, extern), say so — easy to change.
2. herolib_core::base vs renaming Kind → BinaryKind — Kind is a generic name; importing it bare pollutes the catalog file's namespace. Should I support use ... as aliasing (use herolib_core::base::Kind as BinaryKind)? My instinct: yes, mirror Rust, but only implement if you confirm — otherwise I just import as-is.
3. Should the catalog continue to own ServiceCategory and BinaryKind enums with different variants (the drift mentioned above)? Or do we want the catalog to be a strict mirror of ServiceToml and drop the catalog-specific variants? The issue body implies the latter — please confirm before I delete those local enums.

Will hold off on implementation until I see a thumbs-up (or counter-proposal) on the syntax + the answer to Q3.

## Design proposal — OSchema external types Picking up #72. Before coding, I'd like to lock the OSchema syntax for declaring "this type lives in an external Rust crate, don't redefine it." Posting here for sign-off. ### Context — what the failure case actually looks like The issue points at `hero_service`. The on-disk file is `crates/hero_service/schemas/catalog/catalog.oschema` (not `services.oschema` — `services.oschema` got renamed to `catalog.oschema` after the refactor that introduced `ServiceDefinition`). Today that file **redeclares** what `herolib_core::base` already owns: | OSchema redeclaration | Canonical (`herolib_core::base`) | |---|---| | `BinaryKind` | `Kind` | | `SocketKind` | `SockType` | | `SocketProtocol` | `Protocol` | | `ServiceCategory` | `Category` | | `SocketDef` | `Socket` | | `BinaryDef` | `Binary` | The closed-set enums even drift (e.g. `BinaryKind` has `App`, `Kind` doesn't; `ServiceCategory` is `tool/framework/infrastructure/example` vs `Category`'s `core/storage/ai/network/ui/tool`). That drift is exactly the maintenance trap #72 calls out. ### Proposed syntax — `use` statements I propose a top-level `use` statement that mirrors Rust's `use` exactly. It's familiar, unambiguous, and trivially translates to the Rust generator's output: ```oschema use herolib_core::base::{Kind, Category, Protocol, SockType, Binary, Socket, ServiceMeta, ServiceToml} # Now `Kind`, `Binary`, etc. refer to the imported types — never redefined locally. # Catalog-owned types still live in this file. ServiceDefinition = { sid: str # SmartID identifier (assigned by OSIS) name: str @index repo: str @index meta: ServiceMeta # ← resolves to herolib_core::base::ServiceMeta binaries: [Binary] # ← resolves to herolib_core::base::Binary schema_files: [str] tags: [str] created_at: otime updated_at: otime } ``` **Single-import form** (for the one-type case) also accepted: ```oschema use herolib_core::base::ServiceToml ``` ### Why `use` and not an attribute on the existing redefinition? I considered an alternative — keep the redeclaration but tag it `@external("herolib_core::base::Binary")`: ```oschema @external("herolib_core::base::Binary") Binary = { name: str, kind: BinaryKind, ... } ``` I'm proposing against it because **the redeclaration is the bug** — the whole point of #72 is that the local shape is a copy that can drift. Letting authors keep writing a shape next to an `@external` annotation invites that drift back in. `use` makes the name visible without ever writing a shape, which is exactly what we want. ### Semantics — what the model + generators do **Parser / model:** - A new top-level statement: `use <crate>::<path>::{Ident, Ident, ...}` or `use <crate>::<path>::Ident`. - Stored on `Schema` as a `Vec<ExternalImport>` (each carries the crate path + imported names). - The name resolver treats an imported name as a valid target for `TypeExprKind::Named(name)` — references compile through with no local definition required. - Duplicate import + local-definition for the same name = parse error ("name `X` is both imported and defined locally"). **Rust generator (`types_generated.rs`):** - For each `use crate::path::{A, B}` → emit `use crate::path::{A, B};` at the top of the file. - The generator's existing per-type emission loop iterates `schema.definitions` — imports are not in there, so imported names are never redefined. **Rust WASM generator (`types_wasm_generated.rs`):** - This is the tricky one — `herolib_core` may not be wasm-friendly. Two reasonable choices: 1. **Emit the same `use` and let cargo sort it out** (simpler — if the external crate isn't wasm-compatible, the user finds out at compile time, which is honest). 2. **Per-target imports**, e.g. `use crate::path::{A, B} for wasm = "..."` — adds syntax for an edge case nobody's hit yet. - I'd ship (1) and only add per-target overrides when someone actually needs it. **OpenRPC generator:** - Imported names emit as `{ "$ref": "#/components/schemas/Name" }` references like any other named type. - The schema for `Name` is emitted as `{ "type": "object", "description": "External type from <crate path>" }` — opaque, but valid JSON Schema. Generators of typed clients (TS, Python) get a black-box type they can pass through. If we ever need a richer description, the external crate can ship its own JSON Schema alongside. **Rhai / JS / Python generators:** - Skip emission for imported names with a comment "external type, not generated here." Anyone who needs a Rhai binding for `Binary` will get one from the upstream crate. ### Acceptance, restated against this design After the change, `hero_service/crates/hero_service/src/catalog/types_generated.rs` will: - Contain `use herolib_core::base::{Kind, Category, Protocol, SockType, Binary, Socket, ServiceMeta, ServiceToml};` near the top. - **Not** contain any `pub enum Kind`, `pub struct Binary`, etc. - Still contain `ServiceDefinition` (the one type `hero_service` legitimately owns). - `cargo build -p hero_service` clean — no duplicates, references resolve through the import. ### Open questions for sign-off 1. **`use` keyword OK?** It collides with nothing in OSchema today (`use` is not a reserved identifier). If you'd rather a less Rust-flavored word (`import`, `extern`), say so — easy to change. 2. **`herolib_core::base` vs renaming `Kind` → `BinaryKind`** — `Kind` is a generic name; importing it bare pollutes the catalog file's namespace. Should I support `use ... as` aliasing (`use herolib_core::base::Kind as BinaryKind`)? My instinct: yes, mirror Rust, but only implement if you confirm — otherwise I just import as-is. 3. **Should the catalog continue to own `ServiceCategory` and `BinaryKind` enums with different variants** (the drift mentioned above)? Or do we want the catalog to be a strict mirror of `ServiceToml` and drop the catalog-specific variants? The issue body implies the latter — please confirm before I delete those local enums. Will hold off on implementation until I see a thumbs-up (or counter-proposal) on the syntax + the answer to Q3.

timur referenced this issue from lhumina_code/hero_skills 2026-05-20 07:35:58 +00:00

oschema skill prescribes manually declaring sid/created_at/updated_at — generator already auto-injects them #275

timur referenced this issue from lhumina_code/hero_service 2026-05-20 07:36:37 +00:00

catalog domain is over-modelled — collapse ServiceDefinition to name + description + interfaces #1

timur commented 2026-05-20 07:37:07 +00:00

Author

Owner

Copy link

Follow-up — corrections + reframed scope

You're right; my first comment got two things wrong:

1. Schema example was sloppy — ServiceDefinition should not declare sid / created_at / updated_at. Those are auto-injected for any [rootobject] and silently swallowed if declared (crates/generator/src/rust/rust_struct.rs:368-389). The oschema skill in hero_skills actively tells authors to declare them, which is wrong — filed as lhumina_code/hero_skills#275.
2. ServiceDefinition itself is over-modelled — the catalog should be a thin name + description + interfaces record, not a parallel ServiceToml. The on-disk service.toml is already the source of truth for binaries / sockets / category / etc.; the catalog just needs enough to locate a service. Filed as lhumina_code/hero_service#1.\n\nOnce hero_service#1 lands, this issue's headline symptom (Kind / Category / Protocol / SockType / Binary / Socket / ServiceMeta redefinitions in types_generated.rs) goes away on its own — no use-import language feature needed for the immediate fix.

What's left of #72

The OSchema use-import feature still has merit as a generic language addition: future schemas that legitimately need to reference canonical types (e.g. a lab plugin schema that takes ServiceToml as a method parameter) shouldn't be forced to redeclare them either. But it's no longer the path to closing this issue's acceptance — it's a separate enhancement.

Proposed: keep #72 open as the umbrella for "no service ever redeclares herolib_core::base types," link it to hero_service#1 for the immediate hero_service fix, and decide separately whether to land the use-import language feature now or defer until there's a second consumer that actually needs it.

Will hold off on any code in this branch until you confirm whether to:

• (a) Close #72 as fixed-by-hero_service#1 and drop the use-import design entirely.
• (b) Proceed with the use-import language feature here in hero_rpc after hero_service#1 lands.
• (c) Land both together — use-import first, then collapse hero_service catalog as the dogfooding consumer.

Worktree at /tmp/hero_rpc_72 on branch issue-72-oschema-external-types is still parked with no commits.

## Follow-up — corrections + reframed scope You're right; my first comment got two things wrong: 1. **Schema example was sloppy** — `ServiceDefinition` should not declare `sid` / `created_at` / `updated_at`. Those are auto-injected for any `[rootobject]` and silently swallowed if declared (`crates/generator/src/rust/rust_struct.rs:368-389`). The `oschema` skill in `hero_skills` actively tells authors to declare them, which is wrong — filed as **https://forge.ourworld.tf/lhumina_code/hero_skills/issues/275**. 2. **`ServiceDefinition` itself is over-modelled** — the catalog should be a thin name + description + interfaces record, not a parallel `ServiceToml`. The on-disk `service.toml` is already the source of truth for `binaries` / `sockets` / `category` / etc.; the catalog just needs enough to locate a service. Filed as **https://forge.ourworld.tf/lhumina_code/hero_service/issues/1**.\n\nOnce hero_service#1 lands, this issue's headline symptom (`Kind` / `Category` / `Protocol` / `SockType` / `Binary` / `Socket` / `ServiceMeta` redefinitions in `types_generated.rs`) **goes away on its own** — no `use`-import language feature needed for the immediate fix. ### What's left of #72 The OSchema `use`-import feature still has merit as a generic language addition: future schemas that legitimately need to reference canonical types (e.g. a `lab` plugin schema that takes `ServiceToml` as a method parameter) shouldn't be forced to redeclare them either. But it's no longer the *path* to closing this issue's acceptance — it's a separate enhancement. **Proposed:** keep #72 open as the umbrella for "no service ever redeclares `herolib_core::base` types," link it to `hero_service#1` for the immediate hero_service fix, and decide separately whether to land the `use`-import language feature now or defer until there's a second consumer that actually needs it. Will hold off on any code in this branch until you confirm whether to: - (a) Close #72 as fixed-by-hero_service#1 and drop the `use`-import design entirely. - (b) Proceed with the `use`-import language feature here in `hero_rpc` after hero_service#1 lands. - (c) Land both together — `use`-import first, then collapse hero_service catalog as the dogfooding consumer. Worktree at `/tmp/hero_rpc_72` on branch `issue-72-oschema-external-types` is still parked with no commits.

timur commented 2026-05-20 07:48:15 +00:00

Author

Owner

Copy link

Going with (a). The real fix lives in hero_service — collapse the catalog so ServiceTomlf shape isn't mirrored at all, and elevate hero_service into a meta-service that bootstraps / refactors / checks / verifies other hero services (using hero_rpc for codegen, hero_skills for guidance, AI where needed).

Tracking that there: lhumina_code/hero_service#1 (expanded scope, comment to follow on that issue). Closing this as superseded; the use-import OSchema language feature is parked unless a second consumer needs it.

Going with (a). The real fix lives in `hero_service` — collapse the catalog so `ServiceTomlf` shape isn't mirrored at all, and elevate `hero_service` into a meta-service that bootstraps / refactors / checks / verifies other hero services (using `hero_rpc` for codegen, `hero_skills` for guidance, AI where needed). Tracking that there: **https://forge.ourworld.tf/lhumina_code/hero_service/issues/1** (expanded scope, comment to follow on that issue). Closing this as superseded; the `use`-import OSchema language feature is parked unless a second consumer needs it.

timur closed this issue 2026-05-20 07:48:16 +00:00

timur referenced this issue from lhumina_code/hero_service 2026-05-20 07:50:23 +00:00

catalog domain is over-modelled — collapse ServiceDefinition to name + description + interfaces #1

timur referenced this issue from lhumina_code/hero_service 2026-05-20 08:09:59 +00:00

catalog domain is over-modelled — collapse ServiceDefinition to name + description + interfaces #1

timur referenced this issue from lhumina_code/hero_skills 2026-05-20 08:18:49 +00:00

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

timur referenced this issue 2026-05-20 08:39:59 +00:00

fix(generator): drop redundant sid-field requirement on root objects (#85) #88

timur referenced this issue from lhumina_code/hero_service 2026-05-20 08:48:33 +00:00

Collapse catalog to a meta-service for managing Hero services (#1) #3

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

development

issue-98-ui-scaffolds

issue-90-phase4-cleanup

issue-96-generated-folders

issue-90-adapter-lowercase-methods

issue-85-rootobj-sid-validation

lab/74-fold-wasm-types

issue-57-recipe-server-e2e

issue-60-method-translators

issue-54-scaffold-extend

development_refactor_list_full

main

development_jan

latest

v0.1.0

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#72

No description provided.