refactor(2/12): add pipeline event types, parsers, and event builders#320
refactor(2/12): add pipeline event types, parsers, and event builders#320cameroncooke wants to merge 1 commit intorefactor/remove-logging-toolsfrom
Conversation
585faf0 to
9a660e0
Compare
7a6f581 to
477dab1
Compare
| const stSummary = parseSwiftTestingRunSummary(line); | ||
| if (stSummary) { | ||
| completedCount = stSummary.executed; | ||
| failedCount = stSummary.failed; | ||
| emitTestProgress(); | ||
| return; |
There was a problem hiding this comment.
Bug: When parsing a test summary, completedCount and failedCount are reset, but skippedCount is not, leading to inconsistent test progress reporting when tests are skipped.
Severity: MEDIUM
Suggested Fix
Update the ParsedTotals interface to include a skipped field. When parsing a summary line, reset the skippedCount to synchronize it with the authoritative completedCount and failedCount values from the summary.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/utils/swift-testing-event-parser.ts#L113-L118
Potential issue: When parsing a test summary line, the code resets `completedCount` and
`failedCount` based on the summary's values but fails to reset `skippedCount`. If
individual skipped tests were counted incrementally before the summary line is
processed, the final `test-progress` event will contain an inconsistent state where the
`skippedCount` is a stale, incremental value, while the other counts are authoritative
summary values. This is because the `ParsedTotals` interface lacks a `skipped` field,
and the logic in both `swift-testing-event-parser.ts` and `xcodebuild-event-parser.ts`
does not account for synchronizing the skip count.
| if (stContinuation) { | ||
| const lastQueuedEntry = Array.from(pendingFailureDiagnostics.values()).at(-1)?.at(-1); | ||
| if (lastQueuedEntry) { | ||
| lastQueuedEntry.message += `\n${stContinuation}`; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Bug: Swift Testing continuation lines are dropped if a test result arrives before its corresponding issue line, as there is no queued diagnostic to append the continuation message to.
Severity: MEDIUM
Suggested Fix
Modify the parser logic to handle continuation lines that arrive when no diagnostic is queued. This could involve temporarily buffering the continuation line until its corresponding issue appears or creating a placeholder diagnostic to attach it to.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/utils/xcodebuild-event-parser.ts#L275-L280
Potential issue: A Swift Testing continuation line (prefixed with '↳') is silently
dropped if it is processed when no failure diagnostics are queued. This occurs in a
specific but valid ordering: a test result line arrives before its corresponding issue
line. The result is processed, and the issue is emitted immediately instead of being
queued. If a continuation line for that issue arrives afterward, the queue is empty, and
the continuation data is discarded, leading to incomplete error messages.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 477dab1. Configure here.
|
|
||
| const slashParts = rawName.split('/').filter(Boolean); | ||
| if (slashParts.length >= 3) { | ||
| return { suiteName: `${slashParts[0]}/${slashParts[1]}`, testName: slashParts[2] }; |
There was a problem hiding this comment.
Test names with 4+ slash parts get truncated
Low Severity
parseRawTestName handles slashParts.length >= 3 by hardcoding only the first two parts as suiteName and the third as testName, discarding any subsequent parts. For deeply nested Swift Testing suites like Module/OuterSuite/InnerSuite/testMethod, this returns suiteName: 'Module/OuterSuite' and testName: 'InnerSuite', losing testMethod entirely. Using slashParts.slice(0, -1).join('/') and slashParts.at(-1) would handle arbitrary depth correctly.
Reviewed by Cursor Bugbot for commit 477dab1. Configure here.
Summary
This is PR 2 of 12 in a stacked PR series that decouples the rendering pipeline from MCP transport. Depends on PR 1 (logging removal). This PR is additive -- it introduces new modules with no changes to existing code.
Introduces the foundational type system and parsing infrastructure for the new rendering pipeline. The core idea: tools communicate their results through structured
PipelineEventobjects rather than constructing MCPToolResponsecontent directly. This decouples what a tool wants to say from how it gets rendered.Architecture
The pipeline event model has three layers:
Event types (
src/types/pipeline-events.ts): A discriminated union of all event kinds --header,status-line,detail-tree,diagnostic-summary,next-steps,progress, etc. Each event is a plain data object with no rendering logic.Parsers that produce events from xcodebuild/Swift Testing output:
xcodebuild-event-parser.ts: Converts raw xcodebuild stdout lines into structured events (build progress, warnings, errors, test results)xcodebuild-line-parsers.ts: Low-level regex-based line classifiers for xcodebuild outputswift-testing-event-parser.ts: Handles Swift Testing's JSON event stream formatswift-testing-line-parsers.ts: Line classifiers for Swift Testing console outputxcodebuild-run-state.ts: Stateful tracker that deduplicates and orders events during a build/test runxcodebuild-error-utils.ts: Error extraction helpersEvent builders (
tool-event-builders.ts): Factory functions (header(),statusLine(),detailTree(), etc.) that construct properly-typed events. These are what tool handlers call.Why this comes first
Every subsequent PR in the stack depends on these types. The parsers are used by the xcodebuild pipeline (PR 4), the event builders are used by every tool handler (PRs 6-9), and the event types flow through the rendering engine (PR 3) and runtime invoker (PR 5).
Stack navigation
Test plan
npx vitest runpasses -- new test files for each parser module