Skip to content

fix: parametric - preserve meta on materialized parametric results#440

Merged
vic merged 1 commit intovic:mainfrom
sini:fix/parametric-meta-carryover
Apr 12, 2026
Merged

fix: parametric - preserve meta on materialized parametric results#440
vic merged 1 commit intovic:mainfrom
sini:fix/parametric-meta-carryover

Conversation

@sini
Copy link
Copy Markdown
Collaborator

@sini sini commented Apr 12, 2026

Parametric sub-aspects defined as bare functions
(foo._.sub = { host, ... }: { nixos = ...; }) lost their foo provider prefix in the resolved aspect tree: when the functor was invoked by applyDeep, the raw user return ({ nixos = ...; }) didn't carry the sub's meta.provider = ["foo"], so downstream aspectPath-based queries saw the aspect as ["sub"] instead of ["foo","sub"].

This was invisible as long as consumers only extracted classModule via aspect.${class} without caring about aspectPath — see deadbugs/issue-413-provider-sub-aspect-function, still passes. It surfaces the moment anything compares aspect references by their full meta.provider ++ [name] identity.

Fix: add a small carryMeta fn result helper in applyDeep that copies meta from the originating fn onto the materialized result when the result doesn't already carry its own. Applied to both:

  • the outer take result (direct-include path)
  • the inner re-recursion on bare-result.includes (parametric-parent path, where a functor returns { includes = [foo._.sub] })

isBareResult is checked against the pre-carryMeta value so the bare-result recursion branch still fires — carrying meta would otherwise mask it and skip the sub-recursion.

Scope kept narrow to applyDeep rather than touching take.carryAttrs globally: a broader fix there affects every take.* call site (including statics.nix and resolve.apply) and breaks unrelated tests that depend on take results being metaless.

Parametric sub-aspects defined as bare functions
(`foo._.sub = { host, ... }: { nixos = ...; }`) lost their `foo`
provider prefix in the resolved aspect tree: when the functor was
invoked by applyDeep, the raw user return (`{ nixos = ...; }`)
didn't carry the sub's `meta.provider = ["foo"]`, so downstream
aspectPath-based queries saw the aspect as `["sub"]` instead of
`["foo","sub"]`.

This was invisible as long as consumers only extracted classModule
via `aspect.${class}` without caring about aspectPath — see
deadbugs/issue-413-provider-sub-aspect-function, still passes. It
surfaces the moment anything compares aspect references by their
full `meta.provider ++ [name]` identity.

Fix: add a small `carryMeta fn result` helper in applyDeep that
copies `meta` from the originating fn onto the materialized result
when the result doesn't already carry its own. Applied to both:

- the outer `take` result (direct-include path)
- the inner re-recursion on bare-result.includes (parametric-parent
  path, where a functor returns `{ includes = [foo._.sub] }`)

`isBareResult` is checked against the pre-carryMeta value so the
bare-result recursion branch still fires — carrying meta would
otherwise mask it and skip the sub-recursion.

Scope kept narrow to applyDeep rather than touching `take.carryAttrs`
globally: a broader fix there affects every take.* call site
(including statics.nix and resolve.apply) and breaks unrelated
tests that depend on take results being metaless.
@sini sini requested review from theutz and vic April 12, 2026 02:00
@sini sini added the allow-ci allow all CI integration tests label Apr 12, 2026
@sini sini self-assigned this Apr 12, 2026
@vic vic added the merge-approved Approved, merge yourself when ready label Apr 12, 2026
@vic vic merged commit 5870c47 into vic:main Apr 12, 2026
58 of 71 checks passed
sini added a commit that referenced this pull request Apr 13, 2026
# feat: `entity.hasAspect <ref>` query method

## What this does

Adds a `.hasAspect` method on context entities (`host`, `user`, `home`,
and any custom entity kind that imports `den.schema.conf`). It answers
"is this aspect structurally present in my resolved tree?" from inside
class-config module bodies:

```nix
den.aspects.impermanence.nixos = { config, host, ... }: lib.mkMerge [
  (lib.mkIf (host.hasAspect <zfs-root>)   { /* zfs impermanence */ })
  (lib.mkIf (host.hasAspect <btrfs-root>) { /* btrfs impermanence */ })
];
```

There are two variants for when the bare form isn't enough:

```nix
host.hasAspect.forClass "nixos" <facter>
user.hasAspect.forAnyClass <agenix-rekey>
```

Identity is compared by `aspectPath` (`meta.provider ++ [name]`), so
provider sub-aspects like `foo._.sub` keep their full path. Refs can be
plain aspect values (`den.aspects.facter`) or `<angle-bracket>` sugar,
they're the same thing after `__findFile` resolves.

Alongside `hasAspect`, this ships a companion `oneOfAspects` adapter.
That's the structural-decision primitive for "prefer A over B when both
are present", which is the thing you actually want when you're tempted
to use `hasAspect` to decide includes (see the guardrails section
below).

## Why

Real patterns that users hit:

- `<impermanence>` config depends on whether `<zfs-root>` or
  `<btrfs-root>` is also configured on the host
- A secrets forward wants to pick `<agenix-rekey>` when present and
  fall back to `<sops-nix>` otherwise
- Library aspects want to gate opt-in behavior on companion aspects

Today these get worked around with `config.*` lookups, hand-maintained
`lib.elem` checks, or structural hacks. `hasAspect` is the first-class
primitive for the read side. `oneOfAspects` is the first-class primitive
for the write side.

## Commits

Each commit is independently reviewable and builds green on its own.

### `fix(parametric): preserve meta on materialized parametric results`

Opened as it's own PR #440 

### `feat(adapters): add collectPaths terminal adapter`

New public terminal adapter. Walks a resolved tree via `filterIncludes`
and returns `{ paths = [ [providerSeg..., name], ... ]; }`, depth-first,
not deduplicated. Tombstones are skipped via the `meta.excluded` check.

Ships with two small helpers exported from the same file: `pathKey`
(slash-joined path key) and `toPathSet` (list of paths to attrset-as-set
for O(1) lookups). Both are used by `hasAspect` and `oneOfAspects`, so
exporting them keeps the two consumers from duplicating the same
one-liners.

### `feat(adapters): add oneOfAspects structural-decision adapter`

`meta.adapter` that keeps the first structurally-present candidate and
tombstones the rest via `excludeAspect`:

```nix
den.aspects.secrets-bundle = {
  includes = [ <agenix-rekey> <sops-nix> ];
  meta.adapter = den.lib.aspects.adapters.oneOfAspects [
    <agenix-rekey>  # preferred
    <sops-nix>      # fallback
  ];
};
```

Complements `excludeAspect` and `substituteAspect`. The three together
cover include-this / exclude-this / swap-this-for-that. Internally it
walks the parent subtree with the raw collector (bypassing
`filterIncludes` so it doesn't re-enter itself), finds which candidates
are present, and folds `excludeAspect` over the losers. No code
duplication with `collectPaths` thanks to the shared helpers from the
previous commit.

### `feat(aspects): add has-aspect library primitives`

New file `nix/lib/aspects/has-aspect.nix` exporting:

- `hasAspectIn { tree; class; ref }` for any resolved tree, not just
  entity contexts
- `collectPathSet { tree; class }` for an attrset-as-set of visible
  paths
- `mkEntityHasAspect { tree; primaryClass; classes }` which builds the
  functor-plus-attrs value attached to entities. Per-class path sets
  are thunk-cached, so repeated calls share one traversal per class

`refKey` validates that its input has both `name` and `meta` before
reaching for `aspectPath`, throwing loudly rather than silently
producing an `<anon>` path key.

### `feat(context): add entity.hasAspect method via den.schema.conf`

The wiring. `modules/context/has-aspect.nix` is a flake-level module
that self-wires into `den.schema.conf` via
`config.den.schema.conf.imports`. Every entity type that imports `conf`
(host, user, home, and any user-defined kind) inherits `.hasAspect`
automatically. Zero changes to `nix/lib/types.nix`.

Class-protocol: prefers `classes` (list), falls back to `[ class ]`,
throws otherwise. Entities without a matching `den.ctx.<kind>` (so no
`config.resolved`) produce a call-time throw. The fallback value
preserves the functor-plus-attrs shape so `forClass` / `forAnyClass`
attribute access doesn't leak a cryptic error before reaching the real
one.

### `test(has-aspect): full regression-class coverage for entity method`

31 tests organized by aspect-construction shape. Every shape that's
produced a recent regression (`#408`, `#413`, `#423`, `#429`) has a
lock-in test. A future regression in `parametric.nix` or
`aspects/types.nix` that breaks any of these shapes trips a `has-aspect`
test before it reaches user code.

Groups cover basic and transitive chains, parametric contexts including
the static and bare-function sub-aspect shapes, factory functions,
provider sub-aspects and identity disambiguation, mutual-provider and
provides chains, `meta.adapter` composition, multi-class users, the
extensibility contract, error cases, and the primary intended use case
(calling `hasAspect` from inside a deferred `nixos` module body).

### `docs(example): add hasAspect + oneOfAspects worked examples`

User-facing pedagogical file in `templates/example/`. Three sections:
reading structure via `host.hasAspect` from a class-config body, writing
structure via `oneOfAspects` as a `meta.adapter`, and an anti-pattern
section explaining why `hasAspect` can't decide an aspect's `includes`
list with a pointer at the adapter library.

## Design guardrails

`hasAspect` is a read-only query on frozen structure. You call it from
inside class-config module bodies (`nixos = ...`, `homeManager = ...`)
or from lazy positions in aspect functor bodies. It's cycle-safe by
construction because by the time deferred class modules evaluate, the
aspect tree has already been resolved and frozen.

What it is not for: deciding an aspect's `includes` list. That's cyclic.
The tree you want to query depends on the decision you want `hasAspect`
to inform. Users who need that reach for `meta.adapter` composed via
`oneOfAspects`, `excludeAspect`, `substituteAspect`, or `filter` /
`filterIncludes`. Those run during the tree walk with full structural
visibility, so they can't cycle. The template example file has an
explicit anti-pattern section with the failing shape and the correct
rewrite.

Two tools, two jobs:

| Need | Tool | When it runs |
|---|---|---|
| Read "is X in my tree?" from module config | `hasAspect` | After the
tree is frozen, inside lazy class-module bodies |
| Decide tree structure based on "is X present?" | `meta.adapter` +
`oneOfAspects` / friends | During the tree walk, with full structural
visibility |

## Test plan

- [x] `just ci` passes 331/331 at branch tip
- [x] Each commit builds and passes tests on its own
- [x] `just fmt` is idempotent across the tree
- [x] The parametric fix doesn't regress `deadbugs/issue-413-*`,
      `deadbugs/issue-423-*`, or `issue-408` tests
- [x] Full regression-class matrix covers every aspect shape that's
      produced a recent bug

## Migration

None. Purely additive.

- `hasAspect` is a new option name, no conflict.
- No signature changes in `parametric.nix`, `resolve.nix`,
`ctx-apply.nix`, `types.nix`, or the other core library files.
- `adapters.nix` gains new exports (`collectPaths`, `oneOfAspects`,
`pathKey`, `toPathSet`). Nothing is renamed or removed.
- `nix/lib/aspects/default.nix` re-exports the new lib functions under
`den.lib.aspects.*` at their canonical paths.
- `modules/context/has-aspect.nix` is a new file, picked up by the
existing `import-tree` flake setup.
- Zero changes to `nix/lib/types.nix`.

## Follow-ups

- Docs pass on when to make aspects non-parametric. Users often reach
for `perHost` / `perUser` wrappers when the aspect has no actual ctx
dependence, which makes structural tooling less reliable than it could
be. Docs-only, separate PR.
- Conditional-include wrapper `onlyIf guard target`. A small adapter
sitting next to `oneOfAspects` that lets users write `includes = [
(onlyIf <zfs-root> <zfs-impermanence>) ]` for "include target iff guard
is structurally present." Same cycle-avoidance trick (decision runs in
the wrapper's own meta.adapter, walks the subtree with the raw
`collectPathsInner` collector so it doesn't re-enter itself). Reuses
every helper this PR exports. Rough spec is written up, scoped as its
own follow-up PR.
@sini sini deleted the fix/parametric-meta-carryover branch April 14, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allow-ci allow all CI integration tests merge-approved Approved, merge yourself when ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants