Repository navigation
fix(recordings): retain workspace and elapsed time in outcomes - #848
polylane[bot] wants to merge 2 commits into
Conversation
…e-save failures Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
|
| step: WorkflowStep, | ||
| ) { | ||
| const { itemId, attempt } = event.payload; | ||
| const startedAt = Date.now(); |
There was a problem hiding this comment.
Resumed attempts underreport latency
If a transcription workflow resumes after waiting or retrying, startedAt is reset for that invocation. The reported duration_ms omits earlier time in the same attempt. This does not block merging, but it makes latency monitoring less accurate.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Artifacts
Executable transcription duration harness
- This exact script loads the changed TypeScript source, simulates initial and resumed invocations, and captures the emitted outcome.
- The executed control emitted 1000 ms after 1000 ms since the event timestamp, showing the baseline matches.
- The executed resumed run replayed three completed steps and emitted 1000 ms after 101000 ms since the event timestamp, confirming underreporting.
| instanceId: event.instanceId, | ||
| itemId, | ||
| schedule: (task) => this.ctx.waitUntil(task), | ||
| workspaceId: null, |
There was a problem hiding this comment.
When transcription or publication fails after the recording is read, its workspace ID is available, but the failure event records workspace_id: null. This does not block merging, but it makes failures harder to locate by workspace.
Artifacts
Executable focused workflow harness
- This exact script loads the workflow and observability source, stubs external services, and runs both failure paths with current or in-memory-fixed code.
Current-code outcomes after successful reads
- The captured command output shows both failures recording a null workspace ID after reading a known ID.
In-memory-fix outcomes after successful reads
- The captured command output shows the same failure paths recording the known workspace ID with the proposed fix.
| @@ -58,6 +73,18 @@ export class RecordingTranscriptionWorkflow extends WorkflowEntrypoint< | |||
| error instanceof Error ? error.message : String(error), | |||
| ), | |||
| ); | |||
| await step.do("record transcription failure", async () => { | |||
There was a problem hiding this comment.
Database errors skip failure reporting
If marking a recording as failed also throws, the workflow never reaches the failure-outcome step. This does not block merging, but it leaves the original transcription failure without a structured outcome when persistence is failing.
Artifacts
Transcription failure fault-injection harness
- This authored script loads the changed workflow and runs a caught transcription failure with and without a mocked persistence rejection, showing the exact executed test.
Workflow output when marking failure succeeds
- The normal-mode command ran in `/home/user/repo` with exit code 0 and captured one emitted failure outcome.
Workflow output during simulated database outage
- The DB-outage command ran in `/home/user/repo` with exit code 0 and captured a workflow rejection with no emitted outcome.
Comments Outside DiffThese findings could not be posted inline.
|
Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
|
@greptile-apps[bot] All three review points held up against the code, and I applied them. The changes are pushed to #848 ( 1. Resumed attempts underreport latency — valid, fixed. 2. Failure events lose workspace — valid, fixed. The catch path hardcoded 3. Database errors skip failure reporting — valid, fixed. The catch awaited Validation: Follow-up (not in this PR): Pushed in 599082a. |
1 similar comment
|
@greptile-apps[bot] All three review points held up against the code, and I applied them. The changes are pushed to #848 ( 1. Resumed attempts underreport latency — valid, fixed. 2. Failure events lose workspace — valid, fixed. The catch path hardcoded 3. Database errors skip failure reporting — valid, fixed. The catch awaited Validation: Follow-up (not in this PR): Pushed in 599082a. |
Generated by Polylane Autofix from a fix run.
Recording transcription failures were invisible to operators: a failed run marked the recording failed in the database and settled without emitting anything, a resumed run reported only its current invocation's work, and a chat thread whose title save failed logged nothing queryable. Failures now emit one structured outcome naming the recording, workspace, and error, and success reports latency measured from when the job was dispatched.
Why this fix
The transcription workflow was the only user-facing pipeline without an operational outcome event, and the sibling extraction workflow already measures duration from the Workflow event timestamp, so reporting elapsed time since the event matches the repository's established contract for a resumed run. A failure now carries the workspace it read instead of null, and the outcome is emitted before the row is marked failed so a persistence outage cannot suppress the signal. The remaining risk is the row's failed status lagging the event by one step on a database outage; the workflow retry still delivers it.
Out of scope
src/features/workspaces/extraction/workspace-file-extraction-workflow.tsrecords its outcome after its "mark extraction failed" step and so has the same persistence-outage ordering; left as follow-up, outside this PR.chat-compaction.tskeeps its bareconsole.error— that summarizer failure is already captured as a chat-generation error.5 files changed (+315/-2)
src/features/workspaces/ai/chat/chat-endpoint.ts: modified, +13/-1src/features/workspaces/recordings/recording-transcription-workflow.test.ts: added, +162/-0src/features/workspaces/recordings/recording-transcription-workflow.ts: modified, +39/-1src/features/workspaces/recordings/workspace-recording-observability.test.ts: added, +67/-0src/features/workspaces/recordings/workspace-recording-observability.ts: added, +34/-0Generated by Polylane. You can ask follow-ups by mentioning @polylane in a comment.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.