ref(crons): Reorganize incident creation / issue occurrence logic

[ShashankFC]

Test 8nn---nReplicated from ai-code-review-evaluation/sentry-greptile#8

[ShashankFC]

@claude

[claude]

Claude finished @ShashankFC's task in 1m 49s —— View job

---

Code Review Complete ✓

Tasks

• Read repository guidelines (CLAUDE.md)
• Review changed files and understand the refactoring
• Analyze code organization and logic
• Provide detailed feedback

---

Summary

This PR successfully refactors the monitor incident creation logic by extracting functionality from mark_failed.py into two new, well-organized modules. The refactoring improves code organization and separation of concerns, making the codebase more maintainable.

What Changed

Extracted to incidents.py:

• mark_failed_threshold() → try_incident_threshold() - handles incident threshold logic and status transitions

Extracted to incident_occurrence.py:

• create_issue_platform_occurrence() → create_incident_occurrence() - creates issue occurrences for incidents
• get_failure_reason() - builds human-readable failure messages
• get_monitor_environment_context() - constructs monitor environment context
• Constants: HUMAN_FAILURE_STATUS_MAP, SINGULAR_HUMAN_FAILURE_MAP

Moved to types.py:

• SimpleCheckIn TypedDict - now shared across modules

Updated mark_failed.py:

• Simplified to focus on marking check-ins as failed
• Delegates to try_incident_threshold() for incident handling

Findings

✅ Strengths

1. Good Separation of Concerns: Each module now has a clear, single responsibility:

   • mark_failed.py - check-in state management
   • incidents.py - incident lifecycle logic
   • incident_occurrence.py - issue occurrence creation and formatting

2. Improved Testability: Smaller, focused modules are easier to test independently

3. Logical Type Organization: Moving SimpleCheckIn to types.py makes it available for reuse across modules

4. Clean Function Renaming: mark_failed_threshold → try_incident_threshold and create_issue_platform_occurrence → create_incident_occurrence are more descriptive names

🔍 Observations & Minor Issues

1. Typo in Docstring (incident_occurrence.py:132)

   """
   Builds a humam readible string from a list of failed check-ins.

   Should be "human readable"

2. Inconsistent Import Style (incident_occurrence.py:34)
   The function imports IssueOccurrence and related classes locally rather than at module level. This is consistent with the original code, so it's likely intentional (possibly to avoid circular imports), but worth noting.

3. Potential Performance Consideration (incidents.py:93-100)
   When creating occurrences, the code fetches MonitorCheckIn objects again from the database even though it already has the data:

   checkins = MonitorCheckIn.objects.filter(id__in=[c["id"] for c in previous_checkins])
   for checkin in checkins:
       create_incident_occurrence(
           previous_checkins,
           checkin,
           incident,
           received=received,
       )

   This could potentially be optimized, though it may be necessary if create_incident_occurrence needs the full MonitorCheckIn object (not just the SimpleCheckIn data). The current implementation appears correct but might create multiple occurrences in a loop, which could be inefficient if there are many check-ins.

4. Type Annotation Clarity (incidents.py:37-38)
   The use of cast() is appropriate here, but a comment explaining why the cast is safe would be helpful for future maintainers.

✓ No Issues Found With

• Logic correctness - the refactoring preserves all original behavior
• Security - no new vulnerabilities introduced
• Error handling - appropriately maintained from original code
• Import organization - clean and minimal

Recommendation

Approve with minor typo fix. The refactoring is well-executed and improves code organization. The typo in the docstring should be fixed, but it doesn't block merging.

---