Dev - #201
Conversation
- Add SKILL.md with 300+ lines of incident response playbook - Implement incident_classifier.py: severity classification and response recommendations - Implement timeline_reconstructor.py: event timeline reconstruction with phase analysis - Implement pir_generator.py: comprehensive PIR generation with multiple RCA frameworks - Add reference documentation: severity matrix, RCA frameworks, communication templates - Add sample data files and expected outputs for testing - All scripts are standalone with zero external dependencies - Dual output formats: JSON + human-readable text - Professional, opinionated defaults based on SRE best practices This POWERFUL-tier skill provides end-to-end incident response capabilities from detection through post-incident review.
Kept our SKILL.md (POWERFUL-tier, 669 lines) over the codex-synced version. Accepted all new files from dev (additional scripts, references, assets).
feat: Add incident-commander skill (POWERFUL tier)
- Comprehensive interview system design toolkit - Interview Loop Designer: generates calibrated loops for any role/level - Question Bank Generator: creates competency-based questions with rubrics - Hiring Calibrator: analyzes interview data for bias and calibration issues - Complete reference materials: competency matrices, bias mitigation, debrief guides - Sample data and expected outputs for testing - Supports all major roles: SWE, PM, Designer, Data, DevOps, Leadership - Zero external dependencies, Python standard library only - Dual output: JSON + human-readable text formats
…designer feat: add interview-system-designer skill
feat: add migration-architect skill
PR Review - Dev to Main MergeSummaryThis PR merges substantial engineering skill additions from dev to main, adding 4 new engineering skills with comprehensive Python tooling, documentation, and reference materials. The changes include ~27K additions across 57 files. ✅ Strengths1. Excellent Documentation Structure
2. Well-Designed Python Scripts
Code Quality Observations:
3. Comprehensive Reference MaterialsEach skill includes detailed reference guides:
4. Production-Ready Assets
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if duration > 0: | ||
| if duration >= 3600: # 1 hour | ||
| scores["sev1"] += 2 | ||
| scores["sev2"] += 1 | ||
| elif duration >= 1800: # 30 minutes | ||
| scores["sev2"] += 2 | ||
| scores["sev3"] += 1 |
There was a problem hiding this comment.
🔴 Duration thresholds use seconds values but input is in minutes, making duration-based severity scoring ineffective
The _classify_severity method compares duration (sourced from duration_minutes at incident_classifier.py:374) against 3600 and 1800, which are clearly seconds-based thresholds (the comments confirm: # 1 hour and # 30 minutes). However, the input is in minutes — the sample data sample_incident_classification.json:6 shows "duration_minutes": 95.
Root Cause and Impact
Because duration_minutes is compared against 3600 (60 hours in minutes) and 1800 (30 hours in minutes), the duration-based severity boost is essentially dead code for any realistic incident. A 95-minute incident would need to be 3600+ minutes (~2.5 days) to trigger the SEV1 boost, or 1800+ minutes (~30 hours) for the SEV2 boost.
Expected: Thresholds should be 60 (1 hour in minutes) and 30 (30 minutes in minutes).
Actual: Thresholds are 3600 and 1800, treating minutes as seconds.
Impact: Duration is never factored into severity classification for any real-world incident duration, producing less accurate classifications.
| if duration > 0: | |
| if duration >= 3600: # 1 hour | |
| scores["sev1"] += 2 | |
| scores["sev2"] += 1 | |
| elif duration >= 1800: # 30 minutes | |
| scores["sev2"] += 2 | |
| scores["sev3"] += 1 | |
| if duration > 0: | |
| if duration >= 60: # 1 hour | |
| scores["sev1"] += 2 | |
| scores["sev2"] += 1 | |
| elif duration >= 30: # 30 minutes | |
| scores["sev2"] += 2 | |
| scores["sev3"] += 1 |
Was this helpful? React with 👍 or 👎 to provide feedback.
| base_actions = self.action_templates[severity].copy() | ||
|
|
||
| # Customize actions based on incident details | ||
| for action in base_actions: | ||
| if severity in ["sev1", "sev2"]: | ||
| action["urgency"] = "immediate" if severity == "sev1" else "high" | ||
| else: | ||
| action["urgency"] = "normal" if severity == "sev3" else "low" | ||
|
|
||
| return base_actions |
There was a problem hiding this comment.
🟡 Shallow list copy mutates original action templates on every call
The _generate_initial_actions method uses self.action_templates[severity].copy() which creates a shallow copy of the list, but the inner dictionaries remain shared references to the originals defined in _load_action_templates().
Root Cause and Impact
When the loop at lines 493-497 sets action["urgency"], it mutates the original template dictionaries stored in self.action_templates. On the first call with severity "sev1", every action dict in self.action_templates["sev1"] gets "urgency": "immediate" permanently added. On a subsequent call with a different severity (e.g., "sev3"), the shallow copy of that severity's templates would get "urgency": "normal" — but since the "sev1" templates were already mutated on the first call, they now permanently contain "urgency": "immediate".
This is a stateful mutation of what should be immutable template data. While IncidentClassifier instances are typically short-lived in the CLI, if the classifier is reused (e.g., in interactive mode at incident_classifier.py:770 which loops), action templates will carry over stale "urgency" keys from prior classifications.
Expected: Each call produces fresh action dictionaries without modifying originals.
Actual: Original template dicts are permanently mutated with urgency keys.
| base_actions = self.action_templates[severity].copy() | |
| # Customize actions based on incident details | |
| for action in base_actions: | |
| if severity in ["sev1", "sev2"]: | |
| action["urgency"] = "immediate" if severity == "sev1" else "high" | |
| else: | |
| action["urgency"] = "normal" if severity == "sev3" else "low" | |
| return base_actions | |
| def _generate_initial_actions(self, severity: str, incident_data: Dict) -> List[Dict]: | |
| """Generate prioritized initial actions based on severity.""" | |
| import copy | |
| base_actions = copy.deepcopy(self.action_templates[severity]) | |
| # Customize actions based on incident details | |
| for action in base_actions: | |
| if severity in ["sev1", "sev2"]: | |
| action["urgency"] = "immediate" if severity == "sev1" else "high" | |
| else: | |
| action["urgency"] = "normal" if severity == "sev3" else "low" | |
| return base_actions |
Was this helpful? React with 👍 or 👎 to provide feedback.
| }, | ||
| "phase_analysis": phase_analysis, | ||
| "key_participants": dict(actors), | ||
| "event_sources": dict(defaultdict(int, {e.source: 1 for e in events})), |
There was a problem hiding this comment.
🔴 event_sources dict comprehension always counts 1 per source, losing actual frequency data
At line 731, event_sources is computed using dict(defaultdict(int, {e.source: 1 for e in events})). The inner dict comprehension {e.source: 1 for e in events} always maps each source to the integer 1, regardless of how many events come from that source.
Root Cause and Impact
The dict comprehension {e.source: 1 for e in events} iterates over all events but, for duplicate source names, later entries simply overwrite earlier ones — always with 1. Wrapping this in defaultdict(int, ...) is meaningless since all values are already explicitly set to 1.
For example, if 10 events come from "datadog" and 5 from "slack", the result would be {"datadog": 1, "slack": 1} instead of {"datadog": 10, "slack": 5}.
Meanwhile, just a few lines above at timeline_reconstructor.py:717-719, the actors dict correctly uses defaultdict(int) with += to count occurrences. The event_sources line should follow the same pattern.
Expected: {"datadog": 10, "slack": 5, ...} — actual event counts per source.
Actual: {"datadog": 1, "slack": 1, ...} — every source shows count of 1.
| "event_sources": dict(defaultdict(int, {e.source: 1 for e in events})), | |
| "event_sources": dict(defaultdict(int, {source: sum(1 for e in events if e.source == source) for source in set(e.source for e in events)})), |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Context
Changes
Testing
ci-quality-gateworkflow will passTesting Details:
Security
Documentation
Reviewers
Related Issues
Fixes #
Closes #
Related to #
Type:
Scope: