refactor(capabilities): inline datamodel structs into parent packages#21
Merged
Conversation
Each capability exposed its wire structs through a parent-package alias
to a `<cap>/datamodel/` subpackage:
// capabilities/blob/add.go
type AddArguments = bdm.AddArgumentsModel
The split solved a real problem: `bindcap.Arguments` requires CBOR codec
methods, those methods are what `cbor-gen` produces, and the parent
package couldn't compile against itself before codegen ran. Putting the
wire structs in a separate package broke that bootstrap cleanly.
The indirection has accumulated costs worth addressing now:
- The aliases are pure renames (Foo = otherpkg.FooModel), which we've
been trying to avoid elsewhere in the codebase.
- The `Model` suffix leaks back through struct field declarations.
`args.Metadata` reads as `Metadata` at the call site, but hover-types
and error messages still surface `MetadataModel` because the field
is declared with the suffixed name. A recent piri refactor surfaced
a confusing type mismatch where the alias chain took a while to
untangle.
- Each capability duplicates the same `datamodel/` boilerplate (a
`gen/` subfolder, generator entry point, generated codecs).
This change inlines each `<cap>/datamodel/` into its parent and drops
the rename aliases. The codegen bootstrap is preserved with a build
tag: binding files carry `//go:build !codegen` and the generator runs
with `-tags codegen`, so during codegen only the wire types compile
(no bindcap dependency). The generator post-processes its output to
re-inject the tag, keeping subsequent runs stale-safe.
The top-level `capabilities/datamodel.UnitModel` becomes
`capabilities.Unit`. Per-op `OK` aliases that resolved to `UnitModel`
are kept as within-package aliases (e.g. `type AddOK = capabilities.Unit`)
to preserve operation-named labels at type-parameter call sites like
`bindexec.Response[*blob.AddOK]`.
Additionally, `capabilities.MustNew` wraps `bindcap.New` and panics on
construction error. The previous `var Foo, _ = bindcap.New(...)` pattern
silently dropped errors that, for hardcoded commands and options, can
only ever be programmer bugs — failing at package init is strictly
safer than discovering it later via nil dereference.
External impact: only the sprue repo imports an inner datamodel package
directly (two test files using
`access/datamodel.CapabilityRequestModel`). Those need to update
alongside the libforge bump — change the import path and drop the
`Model` suffix.
The pdp/ tree gets the same treatment in a follow-up branch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alanshaw
approved these changes
May 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each capability exposed its wire structs through a parent-package alias to a
<cap>/datamodel/subpackage:The split solved a real problem:
bindcap.Argumentsrequires CBOR codec methods, those methods are whatcbor-genproduces, and the parent package couldn't compile against itself before codegen ran. Putting the wire structs in a separate package broke that bootstrap cleanly.The indirection has accumulated costs worth addressing now:
The aliases are pure renames (Foo = otherpkg.FooModel), which we've been trying to avoid elsewhere in the codebase.
The
Modelsuffix leaks back through struct field declarations.args.Metadatareads asMetadataat the call site, but hover-types and error messages still surfaceMetadataModelbecause the field is declared with the suffixed name. A recent piri refactor surfaced a confusing type mismatch where the alias chain took a while to untangle.Each capability duplicates the same
datamodel/boilerplate (agen/subfolder, generator entry point, generated codecs).This change inlines each
<cap>/datamodel/into its parent and drops the rename aliases. The codegen bootstrap is preserved with a build tag: binding files carry//go:build !codegenand the generator runs with-tags codegen, so during codegen only the wire types compile (no bindcap dependency). The generator post-processes its output to re-inject the tag, keeping subsequent runs stale-safe.The top-level
capabilities/datamodel.UnitModelbecomescapabilities.Unit. Per-opOKaliases that resolved toUnitModelare kept as within-package aliases (e.g.type AddOK = capabilities.Unit) to preserve operation-named labels at type-parameter call sites likebindexec.Response[*blob.AddOK].Additionally,
capabilities.MustNewwrapsbindcap.Newand panics on construction error. The previousvar Foo, _ = bindcap.New(...)pattern silently dropped errors that, for hardcoded commands and options, can only ever be programmer bugs — failing at package init is strictly safer than discovering it later via nil dereference.External impact: only the sprue repo imports an inner datamodel package directly (two test files using
access/datamodel.CapabilityRequestModel). Those need to update alongside the libforge bump — change the import path and drop theModelsuffix.