feat: add FIS safe run preview#216
Conversation
- summarize FIS template blast radius and safeguards - flag missing stop conditions, missing role, broad targets, and unbounded selectors - add preview rendering tests and README/docs updates
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (3)**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⚙️ CodeRabbit configuration file
Files:
**/*_test.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⚙️ CodeRabbit configuration file
Files:
internal/services/aws/**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (5)
WalkthroughAdds an FIS Safe Run Preview: a computed FISSafeRunPreview model with risk/warnings and blast-radius summaries, rendered in the template detail UI with updated help and tests, and documented across README, architecture, and project-overview files. ChangesFIS Safe Run Preview Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Complete
This PR successfully implements FIS safe run preview functionality without introducing any defects that block merge. The implementation follows established patterns, includes comprehensive test coverage, and properly handles security considerations for chaos engineering experiments.
Testing Validation:
- ✅ Unit tests cover both guarded and unsafe states
- ✅ Preview logic correctly flags missing safeguards
- ✅ Integration with existing FIS workflow maintains consistency
The feature is ready for merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/services/aws/fis_model.go`:
- Around line 343-345: FISTemplateStopCondition.IsNone currently only treats
"none" as inactive; update it to consider empty/whitespace sources inactive as
well by trimming s.Source and returning true if the trimmed value is empty OR if
strings.EqualFold(trimmed, "none"); adjust the function to use a local trimmed
:= strings.TrimSpace(s.Source) and then return trimmed == "" ||
strings.EqualFold(trimmed, "none") so callers that check IsNone treat blank
sources as inactive.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 65db0ec7-6620-47c8-99d3-77736e8f9ed8
📒 Files selected for processing (11)
README.mddocs/architecture.en.mddocs/architecture.ko.mddocs/project-overview.en.mddocs/project-overview.ko.mdinternal/app/app_test.gointernal/app/help.gointernal/app/screen_fis.gointernal/domain/catalog.gointernal/services/aws/fis_model.gointernal/services/aws/fis_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
docs/**
⚙️ CodeRabbit configuration file
docs/**: Documentation must match implemented behavior. When both English and
Korean docs are updated, verify that they preserve the same meaning.
Files:
docs/architecture.en.mddocs/project-overview.en.mddocs/architecture.ko.mddocs/project-overview.ko.md
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use lipgloss for styled TUI output — column-aligned tables with dimmed labels in Go implementation files
Implement scroll windowing with formula:visibleLines := max(m.height-N, 5)in Go TUI implementation
Files:
internal/domain/catalog.gointernal/app/help.gointernal/app/screen_fis.gointernal/services/aws/fis_model.gointernal/services/aws/fis_test.gointernal/app/app_test.go
⚙️ CodeRabbit configuration file
**/*.go: For Go reviews, look beyond compilation and prioritize nil pointer risks,
context propagation, AWS SDK pagination, error wrapping, deterministic
sorting, and stable table/detail rendering. For new AWS service work,
verify that repository interfaces, model mapping, app integration, and
tests are updated together.
Files:
internal/domain/catalog.gointernal/app/help.gointernal/app/screen_fis.gointernal/services/aws/fis_model.gointernal/services/aws/fis_test.gointernal/app/app_test.go
internal/app/**
⚙️ CodeRabbit configuration file
internal/app/**: For Bubble Tea screen changes, verify message routing, key handling,
filter target resets, height-based windowing, help text, and back/home
navigation against the existing screen patterns.
Files:
internal/app/help.gointernal/app/screen_fis.gointernal/app/app_test.go
README.md
📄 CodeRabbit inference engine (CLAUDE.md)
README.md: When adding, modifying, or deleting features, always updateREADME.mdin parallel with code changes
UpdateCurrently Implemented Featurestable in README.md: add new services/features, update status changes (🚧→✅), remove deleted items
UpdateTUI Key Bindingstable in README.md when key bindings are added, changed, or deleted
UpdateUsagesection in README.md when new CLI commands or flags are added
UpdateConfigurationsection in README.md when configuration format changes
Files:
README.md
⚙️ CodeRabbit configuration file
README.md: Verify that README changes match actual CLI/TUI behavior and that
Currently Implemented Features, TUI Key Bindings, Usage, and
Configuration content stay aligned with code changes.
Files:
README.md
internal/services/aws/**
⚙️ CodeRabbit configuration file
internal/services/aws/**: For AWS integration code, focus on SDK client interface mockability,
paginator usage, nil/empty response handling, AWS pointer conversion,
stable list ordering, and user-facing error messages.
Files:
internal/services/aws/fis_model.gointernal/services/aws/fis_test.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Tests use mock client interfaces (see
rds_test.gopattern) in Go test files
Files:
internal/services/aws/fis_test.gointernal/app/app_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Check that tests cover API errors, mapping edge cases, and navigation
state transitions, not only happy paths. Prefer mock-based tests that do
not depend on external AWS calls.
Files:
internal/services/aws/fis_test.gointernal/app/app_test.go
🔇 Additional comments (14)
docs/architecture.ko.md (1)
214-214: FIS screen-family scope update is accurate.This now explicitly includes the safe-run preview flow and stays consistent with the English architecture doc.
As per coding guidelines, "Documentation must match implemented behavior. When both English and Korean docs are updated, verify that they preserve the same meaning."
internal/domain/catalog.go (1)
113-113: Catalog description now matches the new FIS flow.The updated feature description reflects safe-run preview plus recent history as intended.
docs/project-overview.ko.md (1)
28-28: Korean scope expansion is consistent and complete.The FIS safe-run preview and enriched history/detail scope are documented clearly and stay in sync with the English overview.
As per coding guidelines, "Documentation must match implemented behavior. When both English and Korean docs are updated, verify that they preserve the same meaning."
docs/architecture.en.md (1)
214-214: Architecture screen-family update looks correct.Including safe-run preview in the FIS flow keeps the architecture boundary accurate.
As per coding guidelines, "Documentation must match implemented behavior. When both English and Korean docs are updated, verify that they preserve the same meaning."
docs/project-overview.en.md (1)
28-28: Current Scope update is aligned with the feature behavior.The FIS description now correctly captures safe-run preview plus richer template/history detail.
As per coding guidelines, "Documentation must match implemented behavior. When both English and Korean docs are updated, verify that they preserve the same meaning."
internal/services/aws/fis_model.go (1)
164-212: Safe-run preview derivation is well-structured and deterministic.The computed counts/warnings plus sorted target/warning lists make the detail rendering stable and predictable.
Also applies to: 249-277
internal/app/screen_fis.go (1)
494-538: Safe-run preview rendering is integrated cleanly.The section structure, warning styling, and confirmation-token prompt all align well with the intended safety-review UX.
internal/app/help.go (1)
488-488: Help text update matches the new detail layout.Including safe-run preview in the scroll guidance keeps keyboard help accurate.
internal/services/aws/fis_test.go (2)
160-192: Good guarded-path safety preview test coverage.This validates the core “safe” contract (risk level, counts, and no warnings) with deterministic inputs.
As per coding guidelines,
**/*_test.go: “Check that tests cover API errors, mapping edge cases, and navigation state transitions, not only happy paths.”
194-231: Good unsafe-path warning coverage.This test exercises multiple independent safety flags and confirms inactive
nonestop conditions are excluded from active counts.As per coding guidelines,
**/*_test.go: “Check that tests cover API errors, mapping edge cases, and navigation state transitions, not only happy paths.”internal/app/app_test.go (2)
1148-1169: Safe-run preview happy-path rendering checks are solid.The assertions now cover the added preview section and key safety summary text in template detail.
As per coding guidelines,
internal/app/**: “For Bubble Tea screen changes, verify message routing, key handling, filter target resets, height-based windowing, help text, and back/home navigation against the existing screen patterns.”
1171-1194: Safety warning rendering test is strong and focused.This covers the “review required” state with concrete warning strings plus the future type-to-confirm template ID prompt.
As per coding guidelines,
**/*_test.go: “Check that tests cover API errors, mapping edge cases, and navigation state transitions, not only happy paths.”README.md (2)
347-347: FIS navigation row is aligned with the new template-detail UX.The key/help wording now clearly reflects safe-run preview visibility and detail scrolling behavior.
As per coding guidelines,
README.md: “Verify that README changes match actual CLI/TUI behavior and that Currently Implemented Features, TUI Key Bindings, Usage, and Configuration content stay aligned with code changes.”
361-361: FIS feature paragraph accurately documents safety preview semantics.It captures the blast-radius summary, warning classes, and the future typed template-ID confirmation guard.
As per coding guidelines,
README.md: “Update the FIS section of ‘TUI Navigation’ to explicitly describe the new ‘safe-run preview’ ... Also document the safety UX requirement that any future execution path must require the user to type the template ID to confirm.”
- consider blank stop-condition sources inactive in safe-run preview - add unit coverage for blank and none stop-condition sources
2 participants
Summary
Add a safe-run preview to FIS experiment template detail so users can inspect blast radius and safeguards before any future execution support.
Related Issues
Closes #172
Validation
go test ./internal/services/aws ./internal/app ./internal/domainmake testmake buildgit diff --checkChecklist
docs/branch-naming-harness.mddocs/documentation-harness.md)docs/pages updated if architecture, auth, config, or workflow changedNotes
Summary by CodeRabbit
New Features
Documentation
Tests