Skip to content

fix(recordings): retain workspace and elapsed time in outcomes - #848

Open
polylane[bot] wants to merge 2 commits into
mainfrom
polylane/autofix/g219f3vb1cb7
Open

polylane[bot] wants to merge 2 commits into
mainfrom
polylane/autofix/g219f3vb1cb7

Conversation

@polylane

@polylane polylane Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

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.

flowchart TD
  A["transcription or publish fails"] --> B["catch: record failure outcome"]
  B --> C["mark recording failed"]
  C -->|rejects| D["error propagates to workflow retry; outcome already emitted"]
  C -->|resolves| E["run settles with outcome and failed row"]
Loading

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.ts records 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.ts keeps its bare console.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/-1
  • src/features/workspaces/recordings/recording-transcription-workflow.test.ts: added, +162/-0
  • src/features/workspaces/recordings/recording-transcription-workflow.ts: modified, +39/-1
  • src/features/workspaces/recordings/workspace-recording-observability.test.ts: added, +67/-0
  • src/features/workspaces/recordings/workspace-recording-observability.ts: added, +34/-0

View thread View autofix


Generated by Polylane. You can ask follow-ups by mentioning @polylane in a comment.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

…e-save failures

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
@polylane polylane Bot added the polylane label Sep 28, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in Dev Board Sep 28, 2026
@polylane polylane Bot added the severity:low Polylane autofix severity: low label Sep 28, 2026
@polylane
polylane Bot requested a review from urjitc September 28, 2026 22:33
@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 79778190-6534-4451-80be-e9b17c087af0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit 599082a.

@polylane polylane Bot changed the title feat(observability): record recording transcription outcomes and title-save failures feat(observability): log transcription outcomes and title failures Sep 28, 2026
@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Adds observability logging to transcription and chat workflows.

These reporting gaps do not block merging, but they reduce the usefulness of transcription monitoring.

Findings

  1. P2 Resumed attempts underreport latency ▶
  2. P2 Failure events lose workspace ▶
  3. P2 Database errors skip failure reporting ▶

Summary

The PR adds structured outcomes for recording transcription and structured reporting for chat title-save failures. Operators need transcription outcomes that identify the workspace, measure elapsed attempt time, and remain visible when failure-state persistence fails. Three non-blocking reporting gaps remain: resumed attempts underreport elapsed time, post-read failures omit the known workspace, and a failure-state update error prevents the failure outcome from being recorded.

Reviews (1) · Last reviewed commit: "feat(observability): record recording tr..."

step: WorkflowStep,
) {
const { itemId, attempt } = event.payload;
const startedAt = Date.now();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Initial invocation output

  • The executed control emitted 1000 ms after 1000 ms since the event timestamp, showing the baseline matches.

Resumed invocation output

  • The executed resumed run replayed three completed steps and emitted 1000 ms after 101000 ms since the event timestamp, confirming underreporting.

View artifacts

T-Rex Ran code and verified through T-Rex

instanceId: event.instanceId,
itemId,
schedule: (task) => this.ctx.waitUntil(task),
workspaceId: null,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Failure events lose workspace

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.

View artifacts

T-Rex Ran code and verified through T-Rex

Comment on lines +68 to +76
@@ -58,6 +73,18 @@ export class RecordingTranscriptionWorkflow extends WorkflowEntrypoint<
error instanceof Error ? error.message : String(error),
),
);
await step.do("record transcription failure", async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

View artifacts

T-Rex Ran code and verified through T-Rex

@greptile-apps

greptile-apps Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff

These findings could not be posted inline.

  • P2 Resumed transcription workflows underreport duration ▶

    • Bug
      • A resumed run emits only the time spent in its current invocation, omitting time since the workflow event was created.
    • Cause
      • src/features/workspaces/recordings/recording-transcription-workflow.ts:26 resets startedAt with Date.now() each time run is invoked; line 58 subtracts that value when emitting the outcome.
    • Fix
      • For elapsed time since workflow creation, calculate duration from event.timestamp.getTime() rather than an invocation-local start time.
  • P2 Transcription failure outcome discards the known workspace ID ▶

    • Bug
      • After a successful read, either a transcription or publish failure records workspace_id: null despite the recording having a workspace ID.
    • Cause
      • The catch-path outcome hardcodes workspaceId: null at src/features/workspaces/recordings/recording-transcription-workflow.ts:84.
    • Fix
      • Retain the workspace ID after a successful read and pass it to the failure outcome, while retaining null when the read itself fails.
  • P2 Persistence failure suppresses the transcription failure outcome ▶

    • Bug
      • After a transcription error is caught, a rejection from failWorkspaceRecording prevents the failure outcome from being emitted, removing the operational signal during a persistence outage.
    • Cause
      • In src/features/workspaces/recordings/recording-transcription-workflow.ts:68-76, the workflow awaits step.do("mark transcription failed") before reaching step.do("record transcription failure").
    • Fix
      • Record the failure outcome independently of whether marking the recording failed succeeds, while preserving the marking error for workflow retry or reporting.

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
@polylane polylane Bot changed the title feat(observability): log transcription outcomes and title failures fix(recordings): retain workspace and elapsed time in outcomes Sep 28, 2026
@polylane

polylane Bot commented Sep 28, 2026

Copy link
Copy Markdown
Author

@greptile-apps[bot] All three review points held up against the code, and I applied them. The changes are pushed to #848 (599082a).

1. Resumed attempts underreport latency — valid, fixed. startedAt was an invocation-local Date.now(), so a run resumed after a wait or retry reported only its current invocation's work. The baseline is now event.timestamp.getTime(), which is exactly what the sibling WorkspaceFileExtractionWorkflow already does (workspace-file-extraction-workflow.ts, "record extraction outcome"), so the field now means end-to-end latency since the job was dispatched and matches the repository's established contract.

2. Failure events lose workspace — valid, fixed. The catch path hardcoded workspace_id: null even when readWorkspaceRecordingForTranscription had already returned the row. I now retain the read workspaceId in a variable and pass it to both the success and failure outcomes; it stays null only when the read itself fails, so no wrong id is ever reported.

3. Database errors skip failure reporting — valid, fixed. The catch awaited step.do("mark transcription failed") before the outcome step, so a rejecting mark raised before the only structured signal was emitted. I reordered the catch to record the failure outcome first, then mark the row; the mark still runs and its rejection still propagates to the workflow retry. This is safe against duplicates because Cloudflare Workflows memoizes completed steps by name, so a retry replays the already-completed outcome step rather than emitting twice.

Validation: pnpm exec vp check passes (no lint/type errors in 823 files; 853 formatted), and pnpm exec vp test --run src/features/workspaces/recordings passes 5 files / 12 tests, including a new recording-transcription-workflow.test.ts that pins the workspace id and error on the failure outcome, the elapsed-since-event duration on both paths, and that the outcome is emitted even when the mark step rejects.

Follow-up (not in this PR): workspace-file-extraction-workflow.ts records its outcome after its "mark extraction failed" step and so has the same persistence-outage ordering; noted in the PR body as out of scope.

Pushed in 599082a.

1 similar comment
@polylane

polylane Bot commented Sep 28, 2026

Copy link
Copy Markdown
Author

@greptile-apps[bot] All three review points held up against the code, and I applied them. The changes are pushed to #848 (599082a).

1. Resumed attempts underreport latency — valid, fixed. startedAt was an invocation-local Date.now(), so a run resumed after a wait or retry reported only its current invocation's work. The baseline is now event.timestamp.getTime(), which is exactly what the sibling WorkspaceFileExtractionWorkflow already does (workspace-file-extraction-workflow.ts, "record extraction outcome"), so the field now means end-to-end latency since the job was dispatched and matches the repository's established contract.

2. Failure events lose workspace — valid, fixed. The catch path hardcoded workspace_id: null even when readWorkspaceRecordingForTranscription had already returned the row. I now retain the read workspaceId in a variable and pass it to both the success and failure outcomes; it stays null only when the read itself fails, so no wrong id is ever reported.

3. Database errors skip failure reporting — valid, fixed. The catch awaited step.do("mark transcription failed") before the outcome step, so a rejecting mark raised before the only structured signal was emitted. I reordered the catch to record the failure outcome first, then mark the row; the mark still runs and its rejection still propagates to the workflow retry. This is safe against duplicates because Cloudflare Workflows memoizes completed steps by name, so a retry replays the already-completed outcome step rather than emitting twice.

Validation: pnpm exec vp check passes (no lint/type errors in 823 files; 853 formatted), and pnpm exec vp test --run src/features/workspaces/recordings passes 5 files / 12 tests, including a new recording-transcription-workflow.test.ts that pins the workspace id and error on the failure outcome, the elapsed-since-event duration on both paths, and that the outcome is emitted even when the mark step rejects.

Follow-up (not in this PR): workspace-file-extraction-workflow.ts records its outcome after its "mark extraction failed" step and so has the same persistence-outage ordering; noted in the PR body as out of scope.

Pushed in 599082a.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

polylane severity:low Polylane autofix severity: low

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant