Skip to content

feat(marqs): optimize batch context fetching in sharedQueueConsumer #4466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
deepshekhardas wants to merge 3 commits into from
Closed

feat(marqs): optimize batch context fetching in sharedQueueConsumer #4466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b9289d
fix: explicitly set machineConfig to null when task machine is remove…
Feb 14, 2026
d6dca75
fix: apply consistent ?? null handling to all Json? fields in Backgro…
Feb 14, 2026
152c383
feat(marqs): optimize batch context fetching in sharedQueueConsumer
Feb 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export class SharedQueueConsumer {

console.log("✅ Started the SharedQueueConsumer");

this.#doWork().finally(() => {});
this.#doWork().finally(() => { });

@devin-ai-integration devin-ai-integration Bot Aug 2, 2026

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.

🟡 Code committed without running the required formatter

Empty callback bodies were rewritten with a stray space inside the braces (() => { } at apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts:265 and again at line 420) instead of the formatter's output, so the committed file no longer matches the repository's enforced formatting.

Impact: Formatting checks / pnpm run format will rewrite these lines, adding noise to the diff and to subsequent PRs.

Prettier rule from AGENTS.md and the exact deviations

AGENTS.md ("Coding style") states: "Formatting is enforced using Prettier. Run pnpm run format before committing." Prettier always emits () => {} for an empty arrow body, never () => { }. Both occurrences (apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts:265 and apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts:420) are purely incidental reformatting unrelated to the PR's purpose.

Suggested change
this.#doWork().finally(() => { });
this.#doWork().finally(() => {});
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}

#endCurrentSpan() {
Expand Down Expand Up @@ -417,7 +417,7 @@ export class SharedQueueConsumer {
span.end();

setTimeout(() => {
this.#doWork().finally(() => {});
this.#doWork().finally(() => { });
}, nextInterval);
}
});
Expand Down Expand Up @@ -620,8 +620,8 @@ export class SharedQueueConsumer {
return existingTaskRun.lockedById
? await getWorkerDeploymentFromWorkerTask(existingTaskRun.lockedById)
: existingTaskRun.lockedToVersionId
? await getWorkerDeploymentFromWorker(existingTaskRun.lockedToVersionId)
: await findCurrentWorkerDeployment({
? await getWorkerDeploymentFromWorker(existingTaskRun.lockedToVersionId)
: await findCurrentWorkerDeployment({
environmentId: existingTaskRun.runtimeEnvironmentId,
type: "V1",
});
Expand Down Expand Up @@ -1650,6 +1650,12 @@ export const AttemptForExecutionGetPayload = {
maxDurationInSeconds: true,
tags: true,
taskEventStore: true,
batch: {
select: {
id: true,
friendlyId: true,
},
},
},
},
queue: {
Expand Down Expand Up @@ -1754,7 +1760,11 @@ class SharedQueueTasks {
slug: attempt.runtimeEnvironment.project.slug,
name: attempt.runtimeEnvironment.project.name,
},
batch: undefined, // TODO: Removing this for now until we can do it more efficiently
batch: attempt.taskRun.batch
? {
id: attempt.taskRun.batch.friendlyId,
}
: undefined,
Comment on lines +1763 to +1767

@devin-ai-integration devin-ai-integration Bot Aug 2, 2026

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.

🟡 New conditional block is indented differently from what the project formatter produces

The newly added conditional value is written with hand-rolled indentation (batch: attempt.taskRun.batch ? { ... } : undefined at apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts:1763-1767) rather than the layout the required formatter produces, so the committed file is not formatter-clean.

Impact: The formatting check will flag the file and re-indent these lines, producing avoidable churn.

Expected Prettier output and other affected spots

AGENTS.md requires running pnpm run format before committing. Prettier indents an object literal inside a ternary branch so its members align under the opening brace:

  batch: attempt.taskRun.batch
    ? {
        id: attempt.taskRun.batch.friendlyId,
      }
    : undefined,

The same unformatted-indentation issue appears in the reflowed deployment ternary (apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts:622-627) and in the getResumePayload return type union (apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts:1913-1915), both of which are unrelated reformatting of untouched code.

Suggested change
batch: attempt.taskRun.batch
? {
id: attempt.taskRun.batch.friendlyId,
}
: undefined,
batch: attempt.taskRun.batch
? {
id: attempt.taskRun.batch.friendlyId,
}
: undefined,
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

worker: {
id: attempt.backgroundWorkerId,
contentHash: attempt.backgroundWorker.contentHash,
Expand Down Expand Up @@ -1900,9 +1910,9 @@ class SharedQueueTasks {

async getResumePayload(attemptId: string): Promise<
| {
execution: V3ProdTaskRunExecution;
completion: TaskRunExecutionResult;
}
execution: V3ProdTaskRunExecution;
completion: TaskRunExecutionResult;
}
| undefined
> {
const attempt = await prisma.taskRunAttempt.findFirst({
Expand Down
8 changes: 4 additions & 4 deletions apps/webapp/app/v3/services/createBackgroundWorker.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,14 @@ async function createWorkerTask(
description: task.description,
filePath: task.filePath,
exportName: task.exportName,
retryConfig: task.retry,
queueConfig: task.queue,
machineConfig: task.machine,
retryConfig: task.retry ?? null,
queueConfig: task.queue ?? null,
machineConfig: task.machine ?? null,
triggerSource: task.triggerSource === "schedule" ? "SCHEDULED" : "STANDARD",
fileId: tasksToBackgroundFiles?.get(task.id) ?? null,
maxDurationInSeconds: task.maxDuration ? clampMaxDuration(task.maxDuration) : null,
queueId: queue.id,
payloadSchema: task.payloadSchema as any,
payloadSchema: (task.payloadSchema as any) ?? null,
Comment on lines +277 to +284

@devin-ai-integration devin-ai-integration Bot Aug 2, 2026

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.

🟡 Pull request bundles an unrelated change alongside the stated feature

A separate, unrelated change to how missing task configuration is stored (retryConfig/queueConfig/payloadSchema defaults at apps/webapp/app/v3/services/createBackgroundWorker.server.ts:277-284) is included in a pull request whose stated purpose is optimizing batch context fetching, so the review scope covers two independent issues.

Impact: Reviewers must evaluate two unrelated changes at once, which the project's contribution rules disallow.

Contribution rule and note on the change itself

CONTRIBUTING.md states: "We only accept PRs that address a single issue. Please do not submit PRs containing multiple unrelated fixes or features. If you have multiple contributions, open a separate PR for each one."

Additionally, these fields are only ever supplied to prisma.backgroundWorkerTask.create() for a brand new row, where an omitted (undefined) value already results in a NULL column, so the ?? null additions do not change behaviour; for nullable Json columns Prisma also expects Prisma.DbNull/Prisma.JsonNull rather than a plain null.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

},
});
} catch (error) {
Expand Down