Merge develop into main

[yimsk]

Summary

• Auto-generate ServiceResourcePath constants (feat: auto-generate ServiceResourcePath constants (#75) #83)
• Modal-based selectors with stack support (refactor: modal-based selectors with stack support (#62) #84)

Changes

• Constants generation: New constants.go files auto-generated for 163 resources with ServiceResourcePath constant
• Modal refactor: Profile/region selectors moved to modal-based UI with stack support for better UX
• CloudFormation rename: cfn/ → cloudformation/ for consistency

[claude]

PR Review: Merge develop into main (#85)

Conducted a comprehensive code review of this large PR (351 files, +2,284/-450 lines). Overall assessment: High quality, safe to merge with minor suggestions for future refinement.

---

Summary

This PR successfully merges two major features from develop:

1. Auto-generate ServiceResourcePath constants (feat: auto-generate ServiceResourcePath constants (#75) #83) - Adds code generation for 163 resources
2. Modal-based selectors with stack support (refactor: modal-based selectors with stack support (#62) #84) - Refactors UI to use modal overlays
3. CloudFormation rename - cfn/ → cloudformation/ for consistency

---

Feature 1: ServiceResourcePath Constants (#83)

Strengths ✅

• Excellent test coverage: Generator has 15 comprehensive test cases covering edge cases
• Build verification: Includes compilation check to catch integration issues early
• Consistent pattern: All 163 generated constants files follow identical structure
• CI verification: Pipeline checks ensure generated files stay in sync
• Clean usage: Constants properly integrated in error messages across all DAOs

Minor Issues ⚠️

1. Package name parsing (Priority: LOW)

• Location: internal/genimports/genimports.go:128-129
• Issue: readPackageName() captures trailing comments/whitespace after package keyword
• Impact: Minimal - works correctly but could be cleaner
• Suggestion:

  if pkg, ok := strings.CutPrefix(line, "package "); ok {
      // Trim everything after first whitespace
      if idx := strings.IndexAny(pkg, " \t"); idx >= 0 {
          pkg = pkg[:idx]
      }
      return pkg
  }

---

Feature 2: Modal-based Selectors (#84)

Strengths ✅

• Proper stack management: Modal stack correctly handles nested modals (Profile Selector → Profile Detail)
• Race condition prevention: Uses incremental refresh IDs to prevent stale async updates
• Generic MultiSelector: Well-designed reusable component with type constraints
• Mouse support: Excellent coordinate translation and bounds checking
• InputCapture interface: Properly distinguishes between navigation and input contexts
• Comprehensive tests: Covers mouse interaction, filtering, empty results, SSO detection

Minor Observations ⚠️

1. Modal content caching (Priority: LOW)

• Location: internal/view/modal.go:179-188 in getModalPosition()
• Issue: Calls ViewString() on every mouse motion event for position calculation
• Impact: Could be expensive for complex modals with frequent mouse movement
• Suggestion: Cache view content or dimensions to reduce redundant renders

2. Modal positioning edge case (Priority: LOW)

• Location: internal/view/modal.go modal positioning logic
• Issue: No upper bounds checking for very large modals on small screens
• Impact: Minimal - width is properly constrained, unlikely to cause issues
• Suggestion: Add upper bounds validation for completeness

---

Feature 3: CloudFormation Rename

Assessment ✅

• Complete migration: All references updated from cfn/ to cloudformation/
• Consistent naming: All BaseDAO instances use correct service name
• Proper constants: Generated files use correct cloudformation/ prefix
• No issues identified

---

Code Quality Assessment

Overall Strengths ✅

1. Consistent code style throughout all 351 files
2. Good separation of concerns - generator, modals, DAOs well-organized
3. Proper error handling - no missing error checks found
4. Clean architecture - follows established patterns
5. No code smells - no TODO/FIXME/HACK comments in critical paths
6. Well-structured tests - good coverage of edge cases

Security Analysis 🔒

• ✅ No injection vulnerabilities (SQL, command, XSS)
• ✅ Proper input sanitization in filter modes
• ✅ Safe use of exec.Command with explicit arguments
• ✅ Generated code contains no user input
• ✅ No credential exposure risks

Performance Considerations ⚡

• ✅ Generator only runs at build time - no runtime impact
• ✅ Modal rendering is efficient
• ⚠️ Minor opportunity: cache modal ViewString() during mouse motion (non-critical)
• ✅ Proper viewport optimization and lazy evaluation

---

Test Coverage

Well-Tested ✅

• Generator logic (15 tests covering edge cases)
• Modal selectors (mouse, keyboard, filtering)
• Profile selector (SSO, toggle, multi-select)
• Modal stack behavior (nested, escape handling)
• Region selector (multi-select, filtering)

Could Add (Optional) 📝

• Modal overlay rendering with complex Unicode characters
• Modal behavior on extremely small terminal sizes
• Integration test verifying all 163 resources have matching constants

---

Recommendations

For This PR

Approve and merge - all issues identified are minor and non-blocking.

For Future PRs (Optional Refinements)

1. Fix package name parser to trim trailing content (LOW priority)
2. Add ViewString caching for modal mouse motion performance (LOW priority)
3. Document modal stack pattern with inline comments explaining the flow (LOW priority)
4. Add integration test ensuring constants generation stays in sync (LOW priority)

---

Conclusion

This is a well-executed PR that:

• ✅ Implements two significant features with clean, maintainable code
• ✅ Maintains consistent code quality across 351 files
• ✅ Includes good test coverage
• ✅ Has no critical bugs or security issues
• ✅ Follows established architectural patterns
• ✅ Includes proper documentation updates

Recommendation: Approve and merge 🚀

The minor issues identified are refinements that can be addressed in future PRs if desired. The code is production-ready.

---

Review conducted using repository documentation (architecture.md, adding-resources.md) and comprehensive code analysis.