Conversation
📝 WalkthroughWalkthroughThe pull request adds trace ingestion and risk routing, adaptive sampling with Python evaluation, issue-agent promotion, incident report persistence, optional fingerprint generation, and expanded monorepo and operational documentation. ChangesIncident monitoring pipeline
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant TraceAPI
participant RiskEngine
participant SamplingQueue
participant SamplingConsumer
participant Evaluator
participant IncidentWorker
participant IssueAgent
Client->>TraceAPI: Submit execution telemetry
TraceAPI->>RiskEngine: Evaluate and normalize risk
RiskEngine-->>TraceAPI: Return severity
TraceAPI->>SamplingQueue: Enqueue healthy sample
SamplingQueue->>SamplingConsumer: Deliver sample
SamplingConsumer->>Evaluator: Run Python evaluator
Evaluator-->>SamplingConsumer: Return classification and failure modes
SamplingConsumer->>IncidentWorker: Promote qualifying incident
IncidentWorker->>IssueAgent: Investigate incident snapshot
IssueAgent-->>IncidentWorker: Return report and issue URL
IncidentWorker-->>IncidentWorker: Persist engineering report and issue URL
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/node-api/src/worker.ts (1)
103-104: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMissing null-check on
evaluation/metadatabefore use — unlike the sibling sampling consumer.If the evaluator returns JSON without an
errorbut also withoutevaluationormetadata(partial/malformed output),evaluation.confidence(line 120) andmetadata.model_version(line 111) throw insidedb.$transaction, aborting persistence of an otherwise-successful evaluation and marking the incidentFAILED.sampling-consumer.tsalready guards this exact case forevaluation(if (!evaluation) { ...; return; }) — this file should apply the same pattern, plus defaultmetadata.🛡️ Proposed fix
- const evaluation = parsed.evaluation as Record<string, unknown>; - const metadata = parsed.metadata as Record<string, unknown>; + const evaluation = parsed.evaluation as Record<string, unknown> | undefined; + const metadata = (parsed.metadata as Record<string, unknown>) ?? {}; + + if (!evaluation) { + await db.incident.update({ + where: { id: incidentId }, + data: { analysis_status: "FAILED" }, + }); + console.error(`[worker] evaluator returned no evaluation for ${incidentId}`); + return; + }🤖 Prompt for 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. In `@apps/node-api/src/worker.ts` around lines 103 - 104, In the worker flow around parsed.evaluation and parsed.metadata, validate that evaluation exists before entering persistence and follow the sibling sampling-consumer.ts behavior by recording the failure and returning when it is absent. Also default metadata to an empty record so metadata.model_version remains safe for partial evaluator output, while preserving normal persistence for complete results.
🧹 Nitpick comments (5)
README.md (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd language identifiers to the fenced blocks.
Both fences trigger Markdownlint MD040 because they omit a language identifier.
README.md#L12-L12: use an appropriate identifier such astextfor the repository tree.README.md#L86-L86: usetextfor the pipeline diagram.🤖 Prompt for 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. In `@README.md` at line 12, Add the text language identifier to both fenced blocks in README.md: the repository tree at lines 12-12 and the pipeline diagram at lines 86-86. Use text for each opening fence to satisfy Markdownlint MD040; no other changes are needed.Source: Linters/SAST tools
apps/node-api/src/python.ts (1)
6-30: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a larger/configurable
maxBuffer.1MB stdout cap on the evaluator/issue-agent subprocess could be hit by verbose LLM output (long reasoning arrays, full engineering reports), causing an otherwise-successful evaluation to fail with
stdout maxBuffer exceeded. Consider raising the limit or making it configurable via env var, consistent withtimeoutMs.♻️ Proposed fix
+const MAX_BUFFER_BYTES = parseInt(process.env.EVALUATOR_MAX_BUFFER_BYTES ?? String(10 * 1024 * 1024), 10); + export function runPythonModule(module: string, inputJson: string, timeoutMs: number): Promise<string> { return new Promise((resolve, reject) => { const child = execFile( PYTHON, ["-m", module], { env: { ...process.env, ...(PYTHONPATH ? { PYTHONPATH } : {}), }, - maxBuffer: 1024 * 1024, + maxBuffer: MAX_BUFFER_BYTES, timeout: timeoutMs, },🤖 Prompt for 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. In `@apps/node-api/src/python.ts` around lines 6 - 30, Update runPythonModule’s execFile options to use a larger, configurable maxBuffer rather than the fixed 1MB limit; read the value from an environment variable with a sensible larger default, while preserving the existing timeoutMs behavior and stdout handling.apps/node-api/src/sampling-consumer.ts (1)
95-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFingerprint hashing duplicates
generateFingerprint's algorithm.
createHash("sha256").update(failureModes.sort().join("|")).digest("hex")mirrors the sha256+"|"-join scheme used by@void-server/incident-fingerprint'sgenerateFingerprint, just applied to LLMfailure_modesstrings instead ofRiskLabel[]. Extracting a shared, generically-typedhashJoin(strings: string[])helper would prevent the two implementations from silently diverging (e.g. if the separator or algorithm changes in one place).🤖 Prompt for 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. In `@apps/node-api/src/sampling-consumer.ts` around lines 95 - 97, Extract the shared SHA-256 joined-string hashing logic from generateFingerprint into a generically reusable hashJoin(strings: string[]) helper, then update both generateFingerprint and the sampling-consumer fingerprint calculation to call it. Preserve the existing "|" separator, sorting behavior, and resulting fingerprint format while eliminating the duplicated createHash implementation.apps/node-api/src/worker.ts (2)
35-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPromotion-gate criteria duplicated across
worker.tsandsampling-consumer.ts. Both files independently implement the same rule —classification === "REAL_INCIDENT" && confidence >= PROMOTION_CONFIDENCE_THRESHOLD && failure_modes.length > 0 && !failure_modes.includes("NONE_DETECTED")— with no shared source of truth, risking silent drift if the criteria change in only one place.
apps/node-api/src/worker.ts#L35-L43: extract the shared gate (keep the"critical-incident"override as an additional case layered on top) into a small helper importable by both files.apps/node-api/src/sampling-consumer.ts#L82-L91: replace the inlinepromotedboolean expression with a call to the same shared helper used byworker.ts.🤖 Prompt for 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. In `@apps/node-api/src/worker.ts` around lines 35 - 43, The promotion criteria are duplicated across both consumers and should use one shared source of truth. In apps/node-api/src/worker.ts lines 35-43, extract the standard classification, confidence, and failure-mode gate into a small importable helper, while keeping the critical-incident override layered in shouldPromoteToIssueAgent; in apps/node-api/src/sampling-consumer.ts lines 82-91, replace the inline promoted expression with that shared helper call.
35-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPromotion gate duplicated with
sampling-consumer.ts.The classification/confidence/failure-mode criteria (minus the critical-incident override) are re-implemented in
sampling-consumer.ts's inlinepromotedcheck, risking drift between the two promotion paths.🤖 Prompt for 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. In `@apps/node-api/src/worker.ts` around lines 35 - 43, Reuse the existing shouldPromoteToIssueAgent function for the promotion decision in sampling-consumer.ts instead of duplicating its classification, confidence, and failure-mode criteria inline. Preserve the critical-incident override and ensure both promotion paths use the same gate.
🤖 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 `@README.md`:
- Around line 161-162: Update the README incident schema field list for
incidents to include the persisted engineering_report and optional issue_url
fields, matching the fields written by the worker flow. Leave the reports schema
documentation unchanged.
- Around line 235-239: Update the “Run Issue Agent CLI (Dev Mode)” command so
VOID_DEV_MODE=1 applies to the python3 -m issue_agent process rather than only
cat; place the assignment on the Python command or export it before the pipeline
while preserving the existing input and PYTHONPATH configuration.
- Around line 217-223: Update the “Issue Agent E2E Monitoring” README
instructions so the Python interpreter used to run e2e_monitor.py matches the
environment where the editable packages were installed; either explicitly create
and install into packages/evaluator/.venv or use the same active python3 for
both pip installation and execution.
- Around line 101-116: Update the README diagram to show the separate
adaptive-sampling stage implemented by sampling-consumer.ts before the BullMQ
incident-analysis worker.ts. Depict sampled healthy executions flowing through
the evaluator, with only qualifying samples promoted to SUSPICIOUS, then routed
to worker.ts for subsequent incident analysis.
- Around line 230-233: Update the evaluator command example to pipe a valid
minimal JSON payload conforming to the evaluator input schema, replacing the
`{...}` placeholders with concrete values or a checked-in fixture reference.
Keep the existing PYTHONPATH and module invocation unchanged.
---
Outside diff comments:
In `@apps/node-api/src/worker.ts`:
- Around line 103-104: In the worker flow around parsed.evaluation and
parsed.metadata, validate that evaluation exists before entering persistence and
follow the sibling sampling-consumer.ts behavior by recording the failure and
returning when it is absent. Also default metadata to an empty record so
metadata.model_version remains safe for partial evaluator output, while
preserving normal persistence for complete results.
---
Nitpick comments:
In `@apps/node-api/src/python.ts`:
- Around line 6-30: Update runPythonModule’s execFile options to use a larger,
configurable maxBuffer rather than the fixed 1MB limit; read the value from an
environment variable with a sensible larger default, while preserving the
existing timeoutMs behavior and stdout handling.
In `@apps/node-api/src/sampling-consumer.ts`:
- Around line 95-97: Extract the shared SHA-256 joined-string hashing logic from
generateFingerprint into a generically reusable hashJoin(strings: string[])
helper, then update both generateFingerprint and the sampling-consumer
fingerprint calculation to call it. Preserve the existing "|" separator, sorting
behavior, and resulting fingerprint format while eliminating the duplicated
createHash implementation.
In `@apps/node-api/src/worker.ts`:
- Around line 35-43: The promotion criteria are duplicated across both consumers
and should use one shared source of truth. In apps/node-api/src/worker.ts lines
35-43, extract the standard classification, confidence, and failure-mode gate
into a small importable helper, while keeping the critical-incident override
layered in shouldPromoteToIssueAgent; in apps/node-api/src/sampling-consumer.ts
lines 82-91, replace the inline promoted expression with that shared helper
call.
- Around line 35-43: Reuse the existing shouldPromoteToIssueAgent function for
the promotion decision in sampling-consumer.ts instead of duplicating its
classification, confidence, and failure-mode criteria inline. Preserve the
critical-incident override and ensure both promotion paths use the same gate.
In `@README.md`:
- Line 12: Add the text language identifier to both fenced blocks in README.md:
the repository tree at lines 12-12 and the pipeline diagram at lines 86-86. Use
text for each opening fence to satisfy Markdownlint MD040; no other changes are
needed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d78ca09d-a795-42ed-bb13-3bdf83a89785
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
README.mdapps/node-api/package.jsonapps/node-api/src/WIRING.mdapps/node-api/src/index.tsapps/node-api/src/python.tsapps/node-api/src/sampling-consumer.tsapps/node-api/src/worker.tspackages/adaptive-sampling/src/queue.tspackages/adaptive-sampling/src/types.tspackages/db/prisma/schema.prismapackages/incident-formation/src/service.tspackages/incident-formation/src/types.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@apps/node-api/src/python.ts`:
- Line 5: Validate the EVALUATOR_MAX_BUFFER value immediately after parsing and
before it is passed to execFile; reject NaN, non-finite, zero, and negative
values, while preserving valid positive configurations and the existing default.
In `@packages/incident-fingerprint/src/incident-fingerprint.ts`:
- Around line 4-8: Update hashJoin to serialize the sorted strings with an
unambiguous encoding such as JSON.stringify instead of sort().join("|"),
preserving deterministic ordering while ensuring elements containing delimiters
cannot produce the same hash input.
- Around line 10-13: Update generateFingerprint and the incident lookup flow in
service.ts to preserve compatibility with fingerprints created using the old
order-sensitive format. Before relying solely on the new fingerprint, add a
legacy lookup path or perform an equivalent backfill/migration, ensuring
existing incidents are found rather than duplicated.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 07715467-f1e4-4713-96f3-5beba72db621
📒 Files selected for processing (6)
README.mdapps/node-api/src/python.tsapps/node-api/src/sampling-consumer.tsapps/node-api/src/worker.tspackages/incident-fingerprint/src/incident-fingerprint.tspackages/incident-fingerprint/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/node-api/src/sampling-consumer.ts
- apps/node-api/src/worker.ts
- README.md
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/incident-fingerprint/tests/incident-fingerprint.test.ts (1)
46-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winProve that legacy fingerprints remain order-sensitive.
This test only compares the legacy result with the current fingerprint. It would still pass if
generateLegacyFingerprintaccidentally sorted labels. Also assert that reversing the labels produces a different legacy fingerprint.Suggested assertion
const labels = [RiskLabel.TOOL_FAILURE, RiskLabel.HIGH_LATENCY]; const legacyFp = generateLegacyFingerprint(labels); +const reversedLegacyFp = generateLegacyFingerprint([...labels].reverse()); expect(legacyFp).toMatch(/^[0-9a-f]{64}$/); +expect(legacyFp).not.toBe(reversedLegacyFp); expect(legacyFp).not.toBe(generateFingerprint(labels));🤖 Prompt for 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. In `@packages/incident-fingerprint/tests/incident-fingerprint.test.ts` around lines 46 - 50, Extend the test “generates legacy order-sensitive fingerprint for backward compatibility” to generate a legacy fingerprint from the reversed labels and assert it differs from the original legacyFp. Keep the existing format and comparison assertions unchanged.packages/incident-formation/tests/formation.test.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact primary-to-legacy lookup sequence.
The test only checks that two lookups occurred. It would pass if the primary fingerprint were computed incorrectly, as long as a second lookup used the legacy value. Assert both calls explicitly.
Suggested assertions
-import { RiskLabel, generateLegacyFingerprint } from "`@void-server/incident-fingerprint`"; +import { + RiskLabel, + generateFingerprint, + generateLegacyFingerprint, +} from "`@void-server/incident-fingerprint`"; + const labels = [RiskLabel.HIGH_LATENCY]; + const primaryFingerprint = generateFingerprint(labels); + const legacyFingerprint = generateLegacyFingerprint(labels); + repo.findByFingerprint = vi.fn().mockImplementation((fp: string) => { - if (fp === generateLegacyFingerprint([RiskLabel.HIGH_LATENCY])) { + if (fp === legacyFingerprint) { return Promise.resolve(existing); } ... + expect(repo.findByFingerprint).toHaveBeenNthCalledWith(1, primaryFingerprint); + expect(repo.findByFingerprint).toHaveBeenNthCalledWith(2, legacyFingerprint);Also applies to: 155-177
🤖 Prompt for 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. In `@packages/incident-formation/tests/formation.test.ts` at line 1, Update the test around the incident fingerprint lookup flow to assert the exact two-call sequence, verifying the first lookup uses the computed primary fingerprint and the second uses generateLegacyFingerprint’s legacy value. Replace the count-only assertion while preserving the existing call order and expected results.
🤖 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.
Nitpick comments:
In `@packages/incident-fingerprint/tests/incident-fingerprint.test.ts`:
- Around line 46-50: Extend the test “generates legacy order-sensitive
fingerprint for backward compatibility” to generate a legacy fingerprint from
the reversed labels and assert it differs from the original legacyFp. Keep the
existing format and comparison assertions unchanged.
In `@packages/incident-formation/tests/formation.test.ts`:
- Line 1: Update the test around the incident fingerprint lookup flow to assert
the exact two-call sequence, verifying the first lookup uses the computed
primary fingerprint and the second uses generateLegacyFingerprint’s legacy
value. Replace the count-only assertion while preserving the existing call order
and expected results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0681b3f3-60f5-487e-bc46-471dbcdc33e5
📒 Files selected for processing (6)
apps/node-api/src/python.tspackages/incident-fingerprint/src/incident-fingerprint.tspackages/incident-fingerprint/src/index.tspackages/incident-fingerprint/tests/incident-fingerprint.test.tspackages/incident-formation/src/service.tspackages/incident-formation/tests/formation.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/incident-fingerprint/src/index.ts
- packages/incident-fingerprint/src/incident-fingerprint.ts
- packages/incident-formation/src/service.ts
This was referenced Jul 25, 2026
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
End-to-end incident pipeline is wired. SDK traces hit
POST /api/traces, we evaluate risk, route healthy runs to adaptive sampling or form incidents, then run the Python evaluator and issue agent to save engineering reports and open issues.New Features
@void-server/node-api: AddedPOST /api/tracesto evaluate risk (@void-server/risk-engine), normalize labels (@void-server/incident-fingerprint), and route to@void-server/adaptive-sampling(HEALTHY) or@void-server/incident-formation(SUSPICIOUS/CRITICAL). IncludesagentStepsandtelemetry.sampling-consumer.tsto consume the sampling queue, run the Pythonevaluator, promote only when REAL_INCIDENT with confidence ≥ threshold (always for critical), fingerprint viahashJoin(failure_modes), and form incidents throughIncidentFormationService.runPythonModule, runs bothevaluatorandissue_agent, promotes using a shared gate, and persistsengineering_reportand optionalissue_url.@void-server/incident-formationnow generates a fingerprint from labels when missing and falls back to a legacy fingerprint to dedupe existing incidents;@void-server/incident-fingerprintaddshashJoinandgenerateLegacyFingerprint. DB addsengineering_report(JSON) andissue_url. Docs expanded (README,apps/node-api/src/WIRING.md).Migration
npm run db:push.npm run dev --workspace=@void-server/node-api,npm run dev:worker --workspace=@void-server/node-api,npm run dev:sampling --workspace=@void-server/node-api.REDIS_URL,EVALUATOR_MODULE(defaultevaluator),ISSUE_AGENT_MODULE(defaultissue_agent),PROMOTION_CONFIDENCE_THRESHOLD(default0.7),PYTHONPATHforpackages/evaluator/src:packages/issue-agent/src. Optional:EVALUATOR_PYTHON,EVALUATOR_PYTHONPATH,EVALUATOR_MAX_BUFFER,ISSUE_AGENT_TIMEOUT_MS.Written for commit e2fc1bc. Summary will update on new commits.
Summary by CodeRabbit
POST /api/tracesto evaluate trace risk and route results to adaptive sampling or incident formation.