Reuse historical incidents during issue grouping - #444
Conversation
|
@codex review |
![superlog-app[bot]](https://avatars.githubusercontent.com/in/3485414?s=60&v=4){kind=small}
| if (err instanceof IssueGroupingFailedError) { | ||
| logger.warn( | ||
| { | ||
| scope: "issue-transitions", |
There was a problem hiding this comment.
logs · blocking — Log grouping retry at error, not warn
A grouping failure blocks issue intake and consumes pg-boss retry budget until it either succeeds or exhausts retries; operators must be able to alert on it. Replace logger.warn with logger.error so the structured log matches the severity of the failure.
| scope: "issue-transitions", | |
| logger.error( | |
| { | |
| scope: "issue-transitions", | |
| issueId: data.issueId, | |
| transition: data.transition, | |
| projectId: data.projectId, | |
| err: err.message, | |
| }, | |
| "issue grouping failed; retrying transition", | |
| ); |
Useful? React with 👍 / 👎.
| eq(schema.incidentEvents.kind, AGENT_PULL_REQUEST_BATCH_RESERVATION_KIND), | ||
| isNull(schema.incidentEvents.processedAt), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
metrics · warning — Add a counter for resolved-incident reopens via grouping
The new joinIssueToIncident path can reopen a resolved incident, but no metric tracks how often this happens; without it operators cannot detect runaway reopen churn or validate that the feature is exercised in production. Increment a low-cardinality counter (e.g. superlog.incidents.reopened, dimension reason=issue_regressed) when previousStatus is non-null.
Useful? React with 👍 / 👎.
|
@codex review |
| if (err instanceof IssueGroupingFailedError) { | ||
| logger.warn( | ||
| { | ||
| scope: "issue-transitions", |
There was a problem hiding this comment.
logs · blocking — Log grouping-failure retries at error, not warn
This path fires when intake cannot group an issue and pg-boss will retry the job; it is an operator-actionable failure (LLM/budget outage blocking incident creation), so it must be logger.error. Using warn here means alerting rules keyed on error severity will miss a sustained grouping outage.
| scope: "issue-transitions", | |
| logger.error( | |
| { | |
| scope: "issue-transitions", | |
| issueId: data.issueId, | |
| transition: data.transition, | |
| projectId: data.projectId, | |
| err: err.message, | |
| }, | |
| "issue grouping failed; retrying transition", | |
| ); |
Useful? React with 👍 / 👎.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b03cc6bb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| where: and( | ||
| eq(schema.incidents.projectId, issue.projectId), | ||
| eq(schema.incidents.status, "open"), | ||
| inArray(schema.incidents.status, ["open", "resolved"]), |
There was a problem hiding this comment.
Including resolved rows in the same last-seen-ordered, capped candidate query can hide valid open join targets: when a project has 200 newer resolved incidents, an older open incident is omitted from every heuristic and LLM grouping pass. A new symptom belonging to that open incident is then classified as standalone, creating a duplicate incident and investigation; prioritize all open incidents or apply a separate limit to resolved history.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in b41f98f: open and resolved incidents now have independent candidate limits, with an integration test proving resolved history cannot evict an open incident.
|
@codex review |
| logger.warn( | ||
| { | ||
| scope: "issue-transitions", | ||
| issueId: data.issueId, |
There was a problem hiding this comment.
logs · blocking — Log grouping retry at error, not warn
A warn here is invisible to alert rules watching for error-level events, yet this path means an issue is stuck in failed state and pg-boss is retrying—an operator must investigate if retries are exhausted. Use logger.error so the structured log is actionable.
| issueId: data.issueId, | |
| logger.error( | |
| { | |
| scope: "issue-transitions", | |
| issueId: data.issueId, | |
| transition: data.transition, | |
| projectId: data.projectId, | |
| err: err.message, | |
| }, | |
| "issue grouping failed; retrying transition", | |
| ); |
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b41f98f7d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ): Promise<void> { | ||
| if (!grouping.failedReason) return; | ||
| await markIssueGrouping(issueId, grouping, repo); | ||
| throw new IssueGroupingFailedError(grouping.failedReason); |
There was a problem hiding this comment.
When pg-boss is unavailable or send fails, createIssueTransitionDispatcher runs this handler inline, so this exception has no queue retry path. Telemetry's flushIssueGroups catches it after the Issue was inserted and treats it as a row failure; later occurrences of that open fingerprint compute a seen transition and never invoke intake again. An LLM failure in this degraded mode therefore leaves the Issue permanently without an Incident, so retain a durable retry or avoid throwing when the transition is running inline.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in aadcb47. Failed or abandoned grouping attempts now remain durable in Postgres, and a minute-scheduled recovery job retries stale unlinked pending or failed issues in isolated batches. The existing queue path still throws for its native retry, while inline fallback failures are recovered by reconciliation.
| const routing = decideIssueArrivalRouting({ shouldInvestigate }); | ||
| if (routing === "none") return intakeResult; |
There was a problem hiding this comment.
When a new signature groups into an Incident with an active agent run, shouldInvestigate is false and this return bypasses the removed appendContextChangeEvent path. The active run's existing issue bundle therefore lacks the new Issue ID, while validateCompleteIncidentIssueOutcomes rejects terminal resolution unless every current linked Issue has an outcome; symptom bursts can consequently prevent an otherwise successful investigation from resolving. Continue appending context to an active run without starting a new investigation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in aadcb47. A newly linked issue now adds a deduplicated context-change event to an existing active run. This path never calls the investigation queue and does nothing when no run is already active.
|
@codex review |
| if (err instanceof IssueGroupingFailedError) { | ||
| logger.warn( | ||
| { | ||
| scope: "issue-transitions", |
There was a problem hiding this comment.
logs · blocking — Use error severity for the grouping-failure rethrow log
This path fires when grouping fails and the job is about to be rejected for pg-boss retry; if retries are exhausted the issue transition is permanently lost, which operators must be able to alert on. warn is invisible to most alert rules and violates the rubric for operations that cannot complete.
| scope: "issue-transitions", | |
| logger.error( | |
| { | |
| scope: "issue-transitions", | |
| issueId: data.issueId, | |
| transition: data.transition, | |
| projectId: data.projectId, | |
| err: err.message, | |
| }, | |
| "issue grouping failed; retrying transition", | |
| ); |
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4aa1801. Grouping failures that reject a queued transition are now logged at error severity.
| } | ||
| await runIssueGroupingRecoverySweep({ | ||
| now: () => new Date(), | ||
| listCandidates: (cutoff) => |
There was a problem hiding this comment.
metrics · warning — Record a counter for recovery sweep outcomes
The sweep returns a retriedCount that is silently discarded; without a metric, operators cannot detect a sweep that consistently finds zero candidates (misconfigured cutoff) or one that is retrying the same issues repeatedly (stuck queue). Add a low-cardinality counter at module scope and record it with the retried count after runIssueGroupingRecoverySweep resolves.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4aa1801. Each recovery sweep now records the successfully recovered issue count in a low-cardinality counter, with a focused adapter test.
|
@codex review |
| scope: "incident-grouping-recovery", | ||
| issue_id: issue.id, | ||
| err: err instanceof Error ? err.message : String(err), | ||
| }, |
There was a problem hiding this comment.
logs · warning — Log the full error object, not just its message, in the recovery sweep
Replace err: err instanceof Error ? err.message : String(err) with err so the structured logger serialises the stack trace and error type; without them an operator cannot distinguish a transient network failure from a permanent schema mismatch when triaging stale-issue recovery failures.
| }, | |
| deps.logger.error( | |
| { | |
| scope: "incident-grouping-recovery", | |
| issue_id: issue.id, | |
| err, | |
| }, | |
| "failed to recover ungrouped issue", | |
| ); |
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Also addressed in 3074858: recovery sweep failures now pass the full error object to the structured logger.
| eq(schema.incidentEvents.kind, AGENT_PULL_REQUEST_BATCH_RESERVATION_KIND), | ||
| isNull(schema.incidentEvents.processedAt), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
metrics · warning — Increment a counter when joinIssueToIncident reopens a resolved incident
Add a superlog.incidents.reopened_total counter increment (with reason: "issue_regressed") inside the if (result.previousStatus) branch so operators can alert on unexpected reopen spikes and distinguish them from new-incident creation in the createdIncident metric.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4aa18016ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| incident: freshIncident, | ||
| createdIncident: false, | ||
| linkedIssue: false, | ||
| shouldInvestigate: false, |
There was a problem hiding this comment.
When a worker is redelivered after the first attempt committed the standalone Incident/link but died before queueAgentRunIfNeeded, this existing-link branch now returns shouldInvestigate: false. The retry therefore skips investigation queueing, and later occurrences of the already-open fingerprint do not re-enter intake, leaving the newly created Incident permanently without an agent run; determine whether the linked Incident already has a run rather than treating every existing link as proof that investigation was started.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3074858. Intake replays now recover queueing only when the durable grouping verdict is standalone and the open Incident has no AgentRun. A grouped historical Incident remains no-investigation even when it has no run, preserving the reuse-only behavior.
|
@codex review |
| { | ||
| scope: "incident-grouping-recovery", | ||
| issue_id: issue.id, | ||
| err, |
There was a problem hiding this comment.
metrics · warning — Add a failure counter for individual recovery-sweep retry errors
Increment a module-scope counter (e.g. superlog.incident_grouping_recovery.retry_failures) with dimension { scope: 'incident-grouping-recovery' } inside the catch block so operators can alert on a persistently broken sweep without parsing log lines. The recovered-issues counter alone cannot distinguish a sweep that retried 0 issues from one that retried 100 and failed all of them.
| err, | |
| deps.logger.error( | |
| { | |
| scope: "incident-grouping-recovery", | |
| issue_id: issue.id, | |
| err, | |
| }, | |
| "failed to recover ungrouped issue", | |
| ); | |
| // increment via injected dep so the pure sweep stays testable | |
| deps.recordRetryFailure?.(); |
Useful? React with 👍 / 👎.
| eq(schema.incidentEvents.kind, AGENT_PULL_REQUEST_BATCH_RESERVATION_KIND), | ||
| isNull(schema.incidentEvents.processedAt), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
logs · blocking — Log at info when joinIssueToIncident reopens a resolved incident
Add a structured info log (with incident_id, issue_id, and previous_status) immediately after the emitIncidentReopened call so operators can reconstruct which grouping decision caused a resolved incident to reappear without querying the database. The incident-reopened event is written to the incidentEvents table but is not surfaced in any log stream.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3074858a32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const groupingState = await repo.findIssueGroupingState(issueId); | ||
| if (groupingState !== "standalone") return false; | ||
| return !(await repo.hasAgentRunForIncident(incident.id)); |
There was a problem hiding this comment.
When standalone intake commits linkIssueToIncident at line 513 but the process dies—or updateIssueGrouping fails—before line 514, the durable state remains pending for the LLM path or the schema default grouped for a no-candidate path. On redelivery, the existing-link branch reaches this helper, which returns false because the state is not yet standalone, so the initial investigation is never queued; findStaleUngroupedIssues cannot repair it because it explicitly excludes issues that already have a link. Fresh evidence beyond the earlier replay fix is this still-non-atomic link/verdict ordering, so the verdict must commit with the link or replay must recognize linked, no-run standalone attempts without relying solely on the finalized state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed at the transaction boundary in 5df6ff9. The grouping verdict is now persisted by joinIssueToIncident in the same database transaction as the link, so a link can no longer commit with a pending or default verdict. The database lifecycle test asserts the link and verdict together, and intake tests assert the atomic grouping payload.
|
@codex review |
| previousStatus, | ||
| }; | ||
| }); | ||
| if (result.previousStatus) { |
There was a problem hiding this comment.
logs · warning — Log when joinIssueToIncident reopens a resolved Incident
Add an info log (with incidentId, issueId, and previousStatus) immediately after the reopen patch is applied inside the transaction so operators can correlate which issue triggered a resolved incident's reopen without a DB query. Without it, a sudden spike in reopened incidents is invisible until someone inspects the incident_reopened event rows directly.
Useful? React with 👍 / 👎.
| const cutoff = new Date(deps.now().getTime() - ISSUE_GROUPING_RETRY_DELAY_MS); | ||
| const candidates = await deps.listCandidates(cutoff); | ||
| let retriedCount = 0; | ||
|
|
There was a problem hiding this comment.
metrics · warning — Record total candidates found alongside recovered count in the sweep
Add a gauge or counter for candidates.length before the retry loop so operators can distinguish an idle sweep (zero candidates) from a sweep that found issues but failed all retries; the existing recovered_issues counter alone cannot surface that difference.
| const cutoff = new Date(deps.now().getTime() - ISSUE_GROUPING_RETRY_DELAY_MS); | |
| const candidates = await deps.listCandidates(cutoff); | |
| if (deps.recordCandidateCount) deps.recordCandidateCount(candidates.length); | |
| let retriedCount = 0; |
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5df6ff9de6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const routing = decideIssueArrivalRouting({ shouldInvestigate }); | ||
| if (routing === "none") { | ||
| await appendIssueToActiveInvestigation(incident, issue, linkedIssue); | ||
| return intakeResult; |
There was a problem hiding this comment.
When a known resolved issue recurs, or a new issue groups into resolved history, intake returns createdIncident: false and shouldInvestigate: false; this branch consequently only attempts to append context to an active run. A resolved incident normally has no active run, so the reopen produces no Slack message at all, whereas the previous recurrence flow posted a new incident root. Projects relying on Slack therefore silently miss renewed production errors; post a reopen/thread notification independently of whether an investigation is active.
Useful? React with 👍 / 👎.
| "New issue to classify:", | ||
| JSON.stringify(input.newIssue, null, 2), | ||
| "", | ||
| `Open incident candidates (${input.candidates.length} total, services: ${services.join(", ") || "-"}, environments: ${environments.join(", ") || "-"}), one per line:`, | ||
| `Incident candidates (${input.candidates.length} total, including open and resolved history; services: ${services.join(", ") || "-"}, environments: ${environments.join(", ") || "-"}), one per line:`, |
There was a problem hiding this comment.
The prompt now says candidates include open and resolved history, but neither this index line nor GroupingCandidateIncident identifies which rows are open. For stackless issues that bypass the frame heuristic, legacy data can contain an open recurrence and a resolved predecessor with nearly identical context; the model may select the resolved row, reopening it while leaving the current incident open and recreating the duplicate aggregates this change is intended to avoid. Include status in each candidate and instruct grouping to prefer the matching open incident.
Useful? React with 👍 / 👎.
| return message.replace( | ||
| /\b\d+(?:\.\d+)?\s?(?:nanoseconds?|microseconds?|milliseconds?|seconds?|minutes?|hours?|days?|bytes?|ns|µs|us|ms|kb|mb|gb|tb|s|m|h|d|b|%)(?![a-z])/gi, | ||
| "<n>", | ||
| ); |
There was a problem hiding this comment.
Replacing every duration, byte size, and percentage with the same <n> token makes semantically different stackless logs collide. For example, otherwise identical limit exceeded: 5s and limit exceeded: 5GB messages now receive the same service/type/body fingerprint, whereas the previous generic normalization retained s versus gb; ingestion will merge distinct failures into one Issue. Normalize units into dimension-specific tokens such as <duration>, <bytes>, and <percent> while still collapsing different values and scales within each dimension.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification
pnpm --filter @superlog/worker run test:incident-intakepnpm --filter @superlog/worker run test:issue-routingpnpm --filter @superlog/worker exec tsx --test src/issue-transitions.test.tspnpm --filter @superlog/worker typecheckpnpm --filter @superlog/db exec tsx --test src/issue-lifecycle.test.tspnpm --filter @superlog/db typecheckpnpm --filter @superlog/fingerprint testpnpm worktree:verifySummary by cubic
Reuses historical incidents during grouping and on recurrences to keep one stable incident per root cause and avoid duplicate agent work. Adds a minute-level recovery sweep for stale grouping and commits grouping verdicts atomically with incident links, plus recovery of missing initial investigations after crashes.
New Features
superlog.incident_grouping_recovery.recovered_issuesmetric.Bug Fixes
IssueGroupingFailedError; the worker rethrows so pg-boss retries instead of creating standalone incidents.Written for commit 5df6ff9. Summary will update on new commits.