fix: make generated outputs deterministic#12
Conversation
thegdsks
left a comment
thegdsks
left a comment
There was a problem hiding this comment.
Solid work — sorting map keys before trimming and the write-only-when-changed pattern are both the right calls. The stable.go helper for preserving generated_at is a nice touch too.
Since this includes the path normalization from #11 as well, I'll merge this one and close #11. Thanks for the contribution!
|
Looks good. Sorting map keys before trimming was needed, and skipping writes on unchanged content is a smart call. The stable.go approach for generated_at is clean too. Since this covers the path changes from #11 as well, I'll go ahead and merge this one. Appreciate the PR! |
There was a problem hiding this comment.
Pull request overview
This PR reduces noisy churn from repeated stacklit generate runs by making generation outputs more deterministic and by skipping writes when the output bytes are unchanged, improving stability especially on Windows.
Changes:
- Normalize walked file paths to forward slashes for OS-stable ignore matching and generated paths.
- Stabilize ordering in generated artifacts (Mermaid node/class ordering; deterministic trimming of capped
TypeDefs). - Avoid rewriting JSON/Mermaid/HTML outputs when the generated bytes match what’s already on disk; preserve
generated_atwhen semantic index content is unchanged.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/walker/walker.go | Normalizes relative paths with filepath.ToSlash for cross-OS stability. |
| internal/walker/walker_test.go | Updates expectations to match forward-slash normalized paths. |
| internal/renderer/stable.go | Adds helper to preserve generated_at when the index content is unchanged. |
| internal/renderer/mermaid.go | Makes Mermaid output deterministic and skips writing when unchanged. |
| internal/renderer/json.go | Preserves generated_at when content is unchanged and skips writing when unchanged. |
| internal/renderer/html.go | Skips writing HTML when unchanged. |
| internal/engine/engine.go | Uses config-driven scan ignore list; deterministic TypeDefs trimming. |
| internal/config/config.go | Adds ScanIgnore() to include outputs in ignore patterns. |
| internal/config/config_test.go | Adds coverage ensuring output paths are included (and normalized) in ScanIgnore(). |
| internal/cli/diff.go | Uses config-driven scan ignore list to exclude generated outputs from diffs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 2. Walk current source files, excluding Stacklit's own generated outputs. | ||
| cfg := config.Load(".") | ||
| files, err := walker.Walk(".", cfg.ScanIgnore()) | ||
| if err != nil { |
There was a problem hiding this comment.
The diff command now loads config to compute scan ignore patterns, but it still reads the index from a hard-coded stacklit.json. If a user overrides output.json in .stacklitrc.json, stacklit diff will read the wrong file or fail. Consider using cfg.Output.JSON (joined to the same root as the scan) for the initial read and error message to keep diff consistent with configured outputs.
| func WriteJSON(idx *schema.Index, path string) error { | ||
| idx.Schema = schemaURL | ||
| idx.GeneratedAt = time.Now().UTC().Format(time.RFC3339) | ||
| idx.StacklitVersion = version | ||
| preserveGeneratedAtIfUnchanged(idx, path) | ||
|
|
||
| data, err := json.MarshalIndent(idx, "", " ") | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if existing, err := os.ReadFile(path); err == nil && bytes.Equal(existing, data) { | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
preserveGeneratedAtIfUnchanged changes WriteJSON semantics by keeping the previous generated_at (and potentially skipping the write entirely). There’s existing test coverage for WriteJSON, but it doesn’t assert this new determinism behavior. Consider adding a test that writes an index twice with unchanged content and verifies (1) generated_at remains the same and (2) the output bytes are unchanged between runs (or that file mtime doesn’t change).
Summary
Why
Repeated stacklit generate runs on unchanged source were still rewriting generated artifacts. On Windows this increases noisy git churn and makes file-lock issues more likely when outputs are open in another process.
This patch makes repeated generation stable when source content has not changed.
Validation