fix: don't coerce factory-function aspects#437
Merged
vic merged 1 commit intodenful:mainfrom Apr 11, 2026
Merged
Conversation
PR denful#410 added coercedProviderType to make `{host, ...}: {nixos = ...}` aspects merge with sibling static `.nixos = ...` defs (fixes denful#408). The predicate was too broad: any non-module function got wrapped into `{ includes = [fn] }`, which silently broke "factory" aspects like: den.aspects.facter = reportPath: { nixos = ...; }; den.aspects.igloo.includes = [ (den.aspects.facter "/path") ]; Coercing turns den.aspects.facter into a full aspect with the default functor. Calling `(facter "/path")` invokes the default functor in a non-static branch with the string as `ctx` — the user's `reportPath` is discarded and the config body never materializes. Narrow the predicate with `lib.functionArgs v != {}`. Context fns have destructured named args (non-empty functionArgs) and still get coerced. Factory fns with a bare positional arg stay typed as providerFnType, whose merge wraps the underlying function via `__functor = _: eth.merge loc defs` so `(aspect arg)` correctly dispatches to the user function. Bisected to d266c3a (PR denful#410). Minimal repro verified broken at that commit and working at v0.15.0. Fix passes the new deadbugs test plus all 278 existing CI tests including deadbugs-issue-408 (context-fn + static merge still works). Fixes denful#429.
sini
changed the title
vic
approved these changes
Apr 11, 2026
sini
added a commit
to sini/den
that referenced
this pull request
Apr 13, 2026
, denful#437, meta carryover Add bare function include handling to resolveDeep — wraps lambdas in minimal aspect envelopes so they participate in recursive resolution. Regression tests cover: - Provider sub-aspect with bare parametric includes (denful#413/denful#423) - Static sub inside parametric parent preserves owned config (denful#426) - Factory function not incorrectly coerced (denful#437) - meta.provider survives deep resolution - 3-level deep parametric nesting
sini
added a commit
to sini/den
that referenced
this pull request
Apr 14, 2026
, denful#437, meta carryover Add bare function include handling to resolveDeep — wraps lambdas in minimal aspect envelopes so they participate in recursive resolution. Regression tests cover: - Provider sub-aspect with bare parametric includes (denful#413/denful#423) - Static sub inside parametric parent preserves owned config (denful#426) - Factory function not incorrectly coerced (denful#437) - meta.provider survives deep resolution - 3-level deep parametric nesting
sini
added a commit
to sini/den
that referenced
this pull request
Apr 14, 2026
, denful#437, meta carryover Add bare function include handling to resolveDeep — wraps lambdas in minimal aspect envelopes so they participate in recursive resolution. Regression tests cover: - Provider sub-aspect with bare parametric includes (denful#413/denful#423) - Static sub inside parametric parent preserves owned config (denful#426) - Factory function not incorrectly coerced (denful#437) - meta.provider survives deep resolution - 3-level deep parametric nesting
sini
added a commit
to sini/den
that referenced
this pull request
Apr 14, 2026
, denful#437, meta carryover Add bare function include handling to resolveDeep — wraps lambdas in minimal aspect envelopes so they participate in recursive resolution. Regression tests cover: - Provider sub-aspect with bare parametric includes (denful#413/denful#423) - Static sub inside parametric parent preserves owned config (denful#426) - Factory function not incorrectly coerced (denful#437) - meta.provider survives deep resolution - 3-level deep parametric nesting
sini
added a commit
to sini/den
that referenced
this pull request
Apr 14, 2026
, denful#437, meta carryover Add bare function include handling to resolveDeep — wraps lambdas in minimal aspect envelopes so they participate in recursive resolution. Regression tests cover: - Provider sub-aspect with bare parametric includes (denful#413/denful#423) - Static sub inside parametric parent preserves owned config (denful#426) - Factory function not incorrectly coerced (denful#437) - meta.provider survives deep resolution - 3-level deep parametric nesting
sini
added a commit
to sini/den
that referenced
this pull request
Apr 17, 2026
- Move tests from batteries/, context/, perf/, home-manager/ subdirs to flat features/ directory - Add fx-specific test suites: fx-aspect, fx-constraints, fx-handlers, fx-resolve, fx-trace, fx-e2e, fx-full-pipeline, fx-identity, fx-includeIf, fx-integration, fx-flag, fx-ctx-apply, fx-ctx-parametric, fx-effectful-resolve, fx-parametric-meta, fx-regressions, fx-adapter-integration - Update existing tests for constraint terminology and provides. alias - Add fxPipeline=false for legacy adapter tests - Add regression reproductions for issues denful#413, denful#423, denful#426, denful#437
vic
added a commit
that referenced
this pull request
Apr 17, 2026
## Summary
Replace den's legacy recursive tree-walking resolution with an
effects-based pipeline. Aspects compile into effectful computations via
`aspectToEffect` — the tree structure emerges from effect composition,
not explicit recursion. All resolution strategy (constraint checking,
dedup, tracing, context dispatch) lives in handlers.
- **`aspectToEffect` compiler** — single function compiles any aspect
into a computation that emits `emit-class`, `register-constraint`,
`emit-include`, and `resolve-complete` effects
- **Handler-owned recursion** — `emit-include` handler checks
constraints and recurses via effectful resume; `into-transition` handler
processes context transitions with `scope.stateful`
- **Constraint system** — `meta.handleWith` / `meta.excludes` replace fx
usage of `meta.adapter`; scoped constraints via includes-chain ancestry;
`exclude`, `substitute`, `filterBy` constructors with subtree/global
variants
- **Includes chain provenance** — `chain-push`/`chain-pop` effects
replace `__parent` string tracking; observable by any handler for
diagrams, scope visualization, composable analysis
- **Module wiring** — `{ lib, den }` only, no `init` function, barrel
`default.nix`; nix-effects accessed as `den.lib.fx`
- **`den.fxPipeline` option** — defaults to `true`; legacy tests using
`resolve.withAdapter` run with `fxPipeline = false`
### Pipeline architecture
```
nix/lib/aspects/fx/
aspect.nix — aspectToEffect compiler
pipeline.nix — mkPipeline, defaultHandlers, fxResolve
identity.nix — path/identity utilities, collectPathsHandler, pathSetHandler
constraints.nix — exclude, substitute, filterBy constructors
includes.nix — includeIf conditional inclusion
trace.nix — structuredTraceHandler, tracingHandler
handlers/
include.nix — emit-include handler (owns recursion)
transition.nix — into-transition handler (scope.stateful)
ctx.nix — constantHandler, ctxSeenHandler
tree.nix — constraintRegistryHandler, chainHandler, classCollectorHandler
```
### Effect protocol
| Effect | Handler responsibility |
|---|---|
| `emit-class` | Accumulate modules by class |
| `emit-include` | Check constraints, recurse via `aspectToEffect` |
| `register-constraint` | Store in registry with scope/ownerChain |
| `check-constraint` | Query registry, first-registered-wins |
| `chain-push` / `chain-pop` | Track includes-path stack |
| `into-transition` | Walk context transitions with scoped handlers |
| `ctx-seen` | Dedup context stages |
| `resolve-complete` | Emit trace entries, accumulate paths |
| `get-path-set` | Return accumulated path set for `includeIf` guards |
| `<arg-name>` | `constantHandler` resumes with context value |
### Compatibility shims
The type system operates at declaration time and cannot be gated on
`config.den.fxPipeline` without circular evaluation:
- `aspect-chain` in `constantHandler` — provider functions from
`providerFnType.merge` create `{ class, aspect-chain }` functors
- `options.nix` uses legacy `ctxApply` — `config.resolved` can't access
`config.den` without circularity
- Legacy adapter tests run with `fxPipeline = false`
### Also in this PR
- `refactor: replace _. alias with provides.` across all
aspect/output/template modules
- `feat: ci-fast recipe` using nix-eval-jobs for parallel test eval
- Test restructuring: `batteries/`, `context/`, `perf/`, `home-manager/`
subdirs flattened to `features/`
- Regression reproductions for issues #413, #423, #426, #437
- Consolidated design spec at `docs/design/fx-pipeline-spec.md`
## Test plan
- [x] `nix develop -c just ci ""` — 488/488 pass
- [x] `nix develop -c just ci-fast` — parallel eval, 488/488 pass
- [x] `nix flake check` — templates, packages, devShells all pass
- [x] Checkmate `system-agnostic` tests — 15/15 pass
- [x] Legacy tests with `fxPipeline = false` verify backward
compatibility
---------
Co-authored-by: Victor Borja <vborja@apache.org>
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
2 participants
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.
PR #410 added coercedProviderType to make
{host, ...}: {nixos = ...}aspects merge with sibling static.nixos = ...defs (fixes #408). The predicate was too broad: any non-module function got wrapped into{ includes = [fn] }, which silently broke "factory" aspects like:Coercing turns den.aspects.facter into a full aspect with the default functor. Calling
(facter "/path")invokes the default functor in a non-static branch with the string asctx— the user'sreportPathis discarded and the config body never materializes.Narrow the predicate with
lib.functionArgs v != {}. Context fns have destructured named args (non-empty functionArgs) and still get coerced. Factory fns with a bare positional arg stay typed as providerFnType, whose merge wraps the underlying function via__functor = _: eth.merge loc defsso(aspect arg)correctly dispatches to the user function.Bisected to d266c3a (PR #410). Minimal repro verified broken at that commit and working at v0.15.0. Fix passes the new deadbugs test plus all 278 existing CI tests including deadbugs-issue-408 (
context-fn + staticmerge still works).Fixes #429.