Add schemas for flakeModules and aspects outputs#55
Add schemas for flakeModules and aspects outputs#55blessuselessk wants to merge 2 commits intoDeterminateSystems:mainfrom
Conversation
This commit adds schema definitions for two new flake output types: - flakeModules: Schema for flake-parts modules that can be imported by other flakes to extend functionality and share configuration patterns. Validates that each module is a function or attribute set. - aspects: Schema for aspect-oriented modules organized by cross-cutting concerns. Supports the flake-aspects pattern where modules are nested as <aspect>.<class>.<module> (e.g., vim-btw.nixos, tiling-desktop.darwin). Both schemas follow the established patterns in the codebase and were tested against real-world usage in vic/vix and dendrix repositories. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add the two new schema types to the list of supported flake output types: - flakeModules: Links to flake.parts documentation - aspects: Links to flake-aspects GitHub repository Both entries are alphabetically sorted in the list. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request adds documentation for new flake output types (aspects and flakeModules) in README.md and introduces two new public schemas in flake.nix to validate flake-parts modules and aspects with inventory-based type checking. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
flake.nix (1)
388-423: Please add regression fixtures for both new schemas.
tests/schemas/flake.nix:1-32still only exercises the samplefooschema, so neitherflakeModulesSchemanor the extra nesting inaspectsSchemais covered by repo-local tests. Since both are exported public schemas, a small fixture for each would make regressions here much easier to catch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` around lines 388 - 423, Add regression fixtures exercising the newly exported flakeModulesSchema and the nested aspectsSchema: update tests/schemas/flake.nix to include a sample flakeModules entry (e.g., a minimal module map like `{ foo = { }; }`) and a sample aspects entry that exercises the extra nesting (e.g., `{ exampleAspect = { nixos = { sample = { }; }; }; }`), then extend the existing test assertions to validate those outputs are accepted by the schema checks; specifically ensure the tests exercise the symbols flakeModulesSchema and aspectsSchema (and the nested class mapping) so regressions in checkModule/evalChecks.isFunctionOrAttrs are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flake.nix`:
- Around line 414-419: The aspectsSchema validation is validating each entire
<class> attrset with checkModule instead of validating each nested <module>, so
modify the builtins.mapAttrs call inside aspectsSchema (the map over classes in
the block that defines what = "${className} module") to descend one level:
replace the direct checkModule module invocation with a nested builtins.mapAttrs
over the module attrset and apply checkModule to each module value. In other
words, inside the mapAttrs that produces the class children, iterate modules and
call checkModule for each module (using the same checkModule function) so
aspects.<aspect>.<class>.<module> entries are validated individually rather than
treating the whole class as a module.
---
Nitpick comments:
In `@flake.nix`:
- Around line 388-423: Add regression fixtures exercising the newly exported
flakeModulesSchema and the nested aspectsSchema: update tests/schemas/flake.nix
to include a sample flakeModules entry (e.g., a minimal module map like `{ foo =
{ }; }`) and a sample aspects entry that exercises the extra nesting (e.g., `{
exampleAspect = { nixos = { sample = { }; }; }; }`), then extend the existing
test assertions to validate those outputs are accepted by the schema checks;
specifically ensure the tests exercise the symbols flakeModulesSchema and
aspectsSchema (and the nested class mapping) so regressions in
checkModule/evalChecks.isFunctionOrAttrs are caught.
| builtins.mapAttrs (aspectName: classes: { | ||
| what = "aspect"; | ||
| children = builtins.mapAttrs (className: module: { | ||
| what = "${className} module"; | ||
| evalChecks.isFunctionOrAttrs = checkModule module; | ||
| }) classes; |
There was a problem hiding this comment.
aspectsSchema stops one level too early.
If the supported shape is aspects.<aspect>.<class>.<module>, Line 416 currently validates the whole <class> attrset with checkModule instead of descending into its modules. That means nested modules are never inventoried individually, and any arbitrary attrset under a class will pass as “valid” because checkModule accepts attrs.
Suggested fix
- builtins.mapAttrs (aspectName: classes: {
- what = "aspect";
- children = builtins.mapAttrs (className: module: {
- what = "${className} module";
- evalChecks.isFunctionOrAttrs = checkModule module;
- }) classes;
+ builtins.mapAttrs (aspectName: classes: {
+ what = "aspect";
+ children = builtins.mapAttrs (className: modules: {
+ what = "${className} modules";
+ children = builtins.mapAttrs (moduleName: module: {
+ what = "${className} module";
+ evalChecks.isFunctionOrAttrs = checkModule module;
+ }) modules;
+ }) classes;
}) output📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| builtins.mapAttrs (aspectName: classes: { | |
| what = "aspect"; | |
| children = builtins.mapAttrs (className: module: { | |
| what = "${className} module"; | |
| evalChecks.isFunctionOrAttrs = checkModule module; | |
| }) classes; | |
| builtins.mapAttrs (aspectName: classes: { | |
| what = "aspect"; | |
| children = builtins.mapAttrs (className: modules: { | |
| what = "${className} modules"; | |
| children = builtins.mapAttrs (moduleName: module: { | |
| what = "${className} module"; | |
| evalChecks.isFunctionOrAttrs = checkModule module; | |
| }) modules; | |
| }) classes; | |
| }) output |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flake.nix` around lines 414 - 419, The aspectsSchema validation is validating
each entire <class> attrset with checkModule instead of validating each nested
<module>, so modify the builtins.mapAttrs call inside aspectsSchema (the map
over classes in the block that defines what = "${className} module") to descend
one level: replace the direct checkModule module invocation with a nested
builtins.mapAttrs over the module attrset and apply checkModule to each module
value. In other words, inside the mapAttrs that produces the class children,
iterate modules and call checkModule for each module (using the same checkModule
function) so aspects.<aspect>.<class>.<module> entries are validated
individually rather than treating the whole class as a module.
There was a problem hiding this comment.
The CodeRabbit suggestion is incorrect — the current schema validates at the correct level. No change needed.
I verified this by reading the flake-aspects source code in detail. Here's the evidence:
1. The README explicitly shows a two-level structure
From flake-aspects README.md:
# flake.aspects — the input
{
vim-btw = {
nixos = ...; # ← this IS the module
darwin = ...;
homeManager = ...;
};
tiling-desktop = {
nixos = ...;
darwin = ...;
};
}The structure is aspects.<aspect>.<class> = <module>, not aspects.<aspect>.<class>.<moduleName> = <module>. Each <class> value is the module itself.
2. The type system confirms it
In nix/types.nix:91-94, the aspectSubmodule type declares:
aspectSubmodule = lib.types.submodule ({ name, config, ... }: {
freeformType = lib.types.lazyAttrsOf lib.types.deferredModule;
...
});The freeform attributes of an aspect (keyed by class name like nixos, darwin, etc.) are typed as lib.types.deferredModule — a single deferred module, not an attrset of modules. This is the type-level proof that aspects.vim-btw.nixos is one module, not a set of modules.
3. The transpose function operates on exactly two levels
nix/default.nix implements a generic 2-level attrset transposition:
# Generic 2-level attribute set transposition
# Swaps parent/child levels: { a.b = 1; } → { b.a = 1; }And nix/aspects.nix uses it to transpose aspects.<aspect>.<class> → modules.<class>.<aspect>:
# Transpose aspects.<aspect>.<class> to modules.<class>.<aspect>
emit = transposed: [{
inherit (transposed) parent child;
value = aspects.${transposed.child}.resolve { class = transposed.parent; };
}];The resolve function returns a single resolved module per (aspect, class) pair — not an attrset of modules.
4. The option definition says it directly
option = lib.mkOption {
default = { };
description = "Aspect definitions organized as <aspect>.<class>";
type = types.aspectsType;
};The description literally says <aspect>.<class> — two levels, not three.
5. What the suggested fix would actually do
CodeRabbit's suggestion adds a third mapAttrs that would try to iterate over the internals of a NixOS/darwin/homeManager module. Since modules are typically functions (e.g., { config, lib, pkgs, ... }: { ... }), calling builtins.mapAttrs on a function would fail with a type error. Even for modules defined as plain attrsets, iterating their keys (like imports, options, config) and calling checkModule on each would be semantically wrong — those are module options, not modules.
6. Consistency with other schemas in this codebase
The aspectsSchema follows the same pattern as bundlersSchema (lines 360-386), which also uses nested children + mapAttrs for a two-level structure (bundlers.<system>.<name>). Both use exactly as many mapAttrs calls as there are nesting levels before reaching the leaf value. The aspects schema has two levels (<aspect>.<class>) and correctly uses two mapAttrs calls.
There was a problem hiding this comment.
@blessuselessk, you're right — I apologize for the incorrect suggestion. The web search confirms your analysis:
The flake.aspects input structure is { vim-btw = { nixos = ...; darwin = ...; homeManager = ...; }; tiling-desktop = { nixos = ...; darwin = ...; }; }, which is a two-level structure where aspects.<aspect>.<class> is the module itself. The library transposes aspect.class into standard Nix module layouts — confirming there is no third nesting level.
Your point about the suggested fix causing runtime failures is also correct: calling builtins.mapAttrs on a function-type module (the common case for NixOS/darwin/homeManager modules) would produce a type error. The current two-mapAttrs implementation is the right approach and is consistent with the bundlersSchema pattern in the codebase.
I've withdrawn my review comment. Thank you for the thorough investigation and detailed explanation.
✏️ Learnings added
Learnt from: blessuselessk
Repo: DeterminateSystems/flake-schemas PR: 55
File: flake.nix:414-419
Timestamp: 2026-03-10T08:24:40.246Z
Learning: In the `flake-aspects` ecosystem (github.com/vic/flake-aspects), the `aspects` flake output has a **two-level** structure: `aspects.<aspect>.<class> = <module>`. The `<class>` value (e.g. `nixos`, `darwin`, `homeManager`) IS the module itself, not a container of modules. The correct schema validation in `flake.nix` uses exactly two nested `builtins.mapAttrs` calls — one for `aspectName` and one for `className` — applying `checkModule` directly to `module`. Adding a third `mapAttrs` level would be wrong and would fail at runtime for function-type modules.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
2 participants
Summary
Adds schema definitions for two flake output types used in the flake-parts and dendrix ecosystems:
flakeModules Schema
flakeModulesoutputcheckModulepattern)aspects Schema
aspectsoutputaspects.<aspect>.<class>.<module>modules.<class>.<aspect>Implementation Details
Both schemas:
checkModulevalidation to ensure modules are functions or attribute setsRebased
This is a rebased version of #45 (from @lessuselesss), updated to resolve merge conflicts with recent additions (bundlers, ociImages rename).
References
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features