feat: add dsp enum parameter for DSP logo overlay (REC-63)#436
Open
recoup-coding-agent wants to merge 1 commit intotestfrom
Open
feat: add dsp enum parameter for DSP logo overlay (REC-63)#436recoup-coding-agent wants to merge 1 commit intotestfrom
recoup-coding-agent wants to merge 1 commit intotestfrom
Conversation
Add `dsp` parameter ("none" | "spotify" | "apple") to the content
creation pipeline. The parameter flows from API validation through
the Slack bot AI parser to the Trigger.dev task payload.
- Add DSP_VALUES constant and DspValue type
- Add dsp field to createContentBodySchema with "none" default
- Add dsp to content prompt agent schema for Slack bot parsing
- Pass dsp through createContentHandler and triggerCreateContent
- Add 4 new tests for dsp validation and prompt extraction
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request adds support for a new Changes
Sequence DiagramsequenceDiagram
participant Slack as Slack Mention
participant Handler as registerOnNewMention
participant Agent as createContentPromptAgent
participant Validator as validateCreateContentBody
participant Trigger as triggerCreateContent
Slack->>Handler: Parse mention with dsp phrase
Handler->>Agent: Call parseContentPrompt()
Agent->>Agent: Extract dsp from prompt<br/>(enum: "none"|"spotify"|"apple")
Agent-->>Handler: Return flags { dsp, ... }
Handler->>Handler: Destructure { dsp } from flags
Handler->>Validator: Pass dsp in payload
Validator->>Validator: Validate dsp against<br/>DSP_VALUES enum
Validator-->>Handler: Return validated object<br/>{ dsp: DspValue, ... }
Handler->>Trigger: Invoke triggerCreateContent<br/>with dsp in payload
Trigger->>Trigger: Forward dsp to task<br/>CREATE_CONTENT_TASK_ID
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/agents/content/handlers/registerOnNewMention.ts (1)
26-27: Optional: surfacedspin thread details/state for better traceability.You pass
dspinto the task correctly; consider also showing/storing it in thread metadata so operators can confirm what was requested.Also applies to: 72-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agents/content/handlers/registerOnNewMention.ts` around lines 26 - 27, The thread metadata/state isn’t recording the parsed dsp value from parseContentPrompt; update the handler to include dsp in the thread details so operators can see what DSP was requested. Specifically, after calling parseContentPrompt(...) (the destructuring that yields dsp) add dsp to the thread/state object updated by the handler (the same place you set batch, captionLength, template, etc.), and also ensure any later code paths that create or update the task (the code around where tasks are enqueued/created) propagate that dsp into thread details/state so it’s visible in UI/logs (apply the same change in the other analogous handler location that handles task creation/update).lib/trigger/triggerCreateContent.ts (1)
11-12: ReuseDspValueinstead of duplicating the DSP union.Line 12 hardcodes values already defined in
lib/content/dspValues.ts, which can drift over time.♻️ Proposed refactor
import { tasks } from "@trigger.dev/sdk"; import { CREATE_CONTENT_TASK_ID } from "@/lib/const"; +import type { DspValue } from "@/lib/content/dspValues"; @@ - /** Which DSP logo to overlay on the video: "none", "spotify", or "apple". */ - dsp: "none" | "spotify" | "apple"; + /** Which DSP logo to overlay on the video. */ + dsp: DspValue;As per coding guidelines "Use constants for repeated values" and "Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/trigger/triggerCreateContent.ts` around lines 11 - 12, The dsp property in triggerCreateContent.ts currently duplicates the DSP union ("none" | "spotify" | "apple"); replace this hardcoded union with the shared type DspValue from lib/content/dspValues.ts by importing DspValue and using it for the dsp field (e.g., change dsp: "none" | "spotify" | "apple" to dsp: DspValue), and update any related type references in triggerCreateContent.ts to use DspValue so the definition stays in sync with the canonical source.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/agents/content/handlers/registerOnNewMention.ts`:
- Around line 26-27: The thread metadata/state isn’t recording the parsed dsp
value from parseContentPrompt; update the handler to include dsp in the thread
details so operators can see what DSP was requested. Specifically, after calling
parseContentPrompt(...) (the destructuring that yields dsp) add dsp to the
thread/state object updated by the handler (the same place you set batch,
captionLength, template, etc.), and also ensure any later code paths that create
or update the task (the code around where tasks are enqueued/created) propagate
that dsp into thread details/state so it’s visible in UI/logs (apply the same
change in the other analogous handler location that handles task
creation/update).
In `@lib/trigger/triggerCreateContent.ts`:
- Around line 11-12: The dsp property in triggerCreateContent.ts currently
duplicates the DSP union ("none" | "spotify" | "apple"); replace this hardcoded
union with the shared type DspValue from lib/content/dspValues.ts by importing
DspValue and using it for the dsp field (e.g., change dsp: "none" | "spotify" |
"apple" to dsp: DspValue), and update any related type references in
triggerCreateContent.ts to use DspValue so the definition stays in sync with the
canonical source.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b73e28ee-1e86-4b1b-be14-92c100025a84
⛔ Files ignored due to path filters (2)
lib/agents/content/__tests__/parseContentPrompt.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/content/__tests__/validateCreateContentBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (6)
lib/agents/content/createContentPromptAgent.tslib/agents/content/handlers/registerOnNewMention.tslib/content/createContentHandler.tslib/content/dspValues.tslib/content/validateCreateContentBody.tslib/trigger/triggerCreateContent.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 8 files
Confidence score: 4/5
- This PR looks safe to merge overall, with low functional risk; the noted issue is a maintainability/type-consistency concern rather than a clear runtime regression.
- Most severe issue:
lib/trigger/triggerCreateContent.tsduplicates the DSP union instead of reusingDspValuefromlib/content/dspValues.ts, which can cause type drift and future mismatches if allowed values change. - Given severity 5/10 (high confidence) and no concrete user-facing break described, this is a minor-to-moderate cleanup item rather than a merge blocker.
- Pay close attention to
lib/trigger/triggerCreateContent.tsandlib/content/dspValues.ts- ensureDspValueis the single source of truth to avoid divergence.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/trigger/triggerCreateContent.ts">
<violation number="1" location="lib/trigger/triggerCreateContent.ts:12">
P2: Use the `DspValue` type from `lib/content/dspValues.ts` instead of duplicating the union inline. This PR already introduces `DspValue` as the single source of truth for valid DSP values—using the inline union here creates a second definition that can drift.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User / Client
participant Slack as Slack Webhook
participant API as Content Handler
participant Agent as LLM Agent (ContentPrompt)
participant Val as Zod Validation
participant Trigger as Trigger.dev (Task Queue)
Note over User, Trigger: Content Creation Flow with DSP Logo Selection
alt Flow A: Natural Language (e.g., Slack)
User->>Slack: "Make a Spotify editorial video"
Slack->>API: registerOnNewMention(message)
API->>Agent: NEW: parseContentPrompt(text)
Note right of Agent: Extracts dsp: "spotify" from text
Agent-->>API: Return prompt flags (inc. dsp)
else Flow B: Structured API Request
User->>API: POST /api/content { "dsp": "apple" }
API->>Val: NEW: validateCreateContentBody(body)
alt Invalid Enum Value (e.g., "tidal")
Val-->>API: Throw 400 Bad Request
API-->>User: 400 Error
else Valid Value or Omitted
Val-->>API: Return ValidatedCreateContentBody
end
end
API->>Trigger: NEW: triggerCreateContent(payload)
Note right of Trigger: Payload includes dsp: "none" | "spotify" | "apple"
Trigger->>Trigger: Execute background content generation<br/>with DSP logo overlay logic
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| /** Controls caption length: "none" skips captions, "short", "medium", or "long". */ | ||
| captionLength: "none" | "short" | "medium" | "long"; | ||
| /** Which DSP logo to overlay on the video: "none", "spotify", or "apple". */ | ||
| dsp: "none" | "spotify" | "apple"; |
There was a problem hiding this comment.
P2: Use the DspValue type from lib/content/dspValues.ts instead of duplicating the union inline. This PR already introduces DspValue as the single source of truth for valid DSP values—using the inline union here creates a second definition that can drift.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/trigger/triggerCreateContent.ts, line 12:
<comment>Use the `DspValue` type from `lib/content/dspValues.ts` instead of duplicating the union inline. This PR already introduces `DspValue` as the single source of truth for valid DSP values—using the inline union here creates a second definition that can drift.</comment>
<file context>
@@ -8,6 +8,8 @@ export interface TriggerCreateContentPayload {
/** Controls caption length: "none" skips captions, "short", "medium", or "long". */
captionLength: "none" | "short" | "medium" | "long";
+ /** Which DSP logo to overlay on the video: "none", "spotify", or "apple". */
+ dsp: "none" | "spotify" | "apple";
/** Whether to upscale image and video for higher quality. */
upscale: boolean;
</file context>
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
dspparameter (none|spotify|apple) to the content creation pipelineDSP_VALUESconstant andDspValuetype inlib/content/dspValues.tsdspfield added to API validation schema, trigger payload, content prompt agent, and Slack mention handlerTest plan
validateCreateContentBodyacceptsdspand defaults tononevalidateCreateContentBodyrejects invalid dsp values with 400parseContentPromptextracts dsp from natural language🤖 Generated with Claude Code
Summary by CodeRabbit