Conversation
Collaborator
Author
I, Jason Bowman, explicitly acknowledge the following:
- [x] My contribution is AI generated and I mention which models/mcp-servers/tools were used.
- [x] I recognize AI generated contribution as my own and am myself legally accountable for it.
- [x] My contribution aligns with Den LICENSE and does not violate any other projects licenses (e.g. GPL)
- [x] I will truthfully answer and provide any requested details on how contribution was generated.Filling this out to show that it's not that scary, I had claude code adapt the test from the bug report for me and write the commit message since it's more descriptive than I can manage. :-) |
PR vic#419 introduced applyDeep to propagate parametric context into bare-result sub-includes. But it called takeFn unconditionally on every sub, including full static aspects whose default functor (withOwn parametric.atLeast) discards owned class configs in a non-static branch. Result: `den.aspects.role._.sub.nixos.x = true` was silently dropped when role was a parametric parent that included its sub-aspect. Gate the inner re-application on canTake.upTo: a static aspect's default functor has empty functionArgs, so upTo is false and the sub is left alone for the static resolve pass. User-provided provider fns (e.g. `{ host, ... }: { nixos = ...; }`) have host in functionArgs, so upTo fires and their config is materialized. This preserves the Fixes vic#423.
sini
force-pushed
the
fix/static-sub-aspect-parametric-include
branch
from
ed50b73 to
72b7f45
Compare
theutz
approved these changes
Apr 10, 2026
| in | ||
| if sr != { } then sr else sub | ||
| ) r.includes; | ||
| # Only recurse into subs whose functor actually consumes this ctx. |
Collaborator
There was a problem hiding this comment.
Thanks for this comment. Very helpful!
sini
added a commit
to sini/den
that referenced
this pull request
Apr 13, 2026
, 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 (vic#413/vic#423) - Static sub inside parametric parent preserves owned config (vic#426) - Factory function not incorrectly coerced (vic#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
, 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 (vic#413/vic#423) - Static sub inside parametric parent preserves owned config (vic#426) - Factory function not incorrectly coerced (vic#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
, 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 (vic#413/vic#423) - Static sub inside parametric parent preserves owned config (vic#426) - Factory function not incorrectly coerced (vic#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
, 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 (vic#413/vic#423) - Static sub inside parametric parent preserves owned config (vic#426) - Factory function not incorrectly coerced (vic#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
, 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 (vic#413/vic#423) - Static sub inside parametric parent preserves owned config (vic#426) - Factory function not incorrectly coerced (vic#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 vic#413, vic#423, vic#426, vic#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 #419 introduced applyDeep to propagate parametric context into bare-result sub-includes. But it called takeFn unconditionally on every sub, including full static aspects whose default functor (withOwn parametric.atLeast) discards owned class configs in a non-static branch. Result:
den.aspects.role._.sub.nixos.x = truewas silently dropped when role was a parametric parent that included its sub-aspect.Gate the inner re-application on canTake.upTo: a static aspect's default functor has empty functionArgs, so upTo is false and the sub is left alone for the static resolve pass. User-provided provider fns (e.g.
{ host, ... }: { nixos = ...; }) have host in functionArgs, so upTo fires and their config is materialized. This preserves the #419 fix while restoring pre-#419 behavior for static sub-aspects.Fixes #423.