Add self-harm/intent + self-harm/instructions OpenAI signals - #535
Merged
Conversation
julietshen
requested review from
cassidyjames,
dom-notion,
juanmrad and
vinaysrao1
as code owners
May 21, 2026 02:31
Contributor
📝 WalkthroughWalkthroughThe change adds OpenAI self-harm instruction and intent signals for text and image inputs. It updates signal contracts, moderation factories, built-in registrations, GraphQL definitions, and client integration routing. ChangesOpenAI Self-Harm Moderation Signal Support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BuiltInSignalFactory
participant OpenAiSelfHarmSignal
participant getOpenAiScores
participant OpenAI moderation API
BuiltInSignalFactory->>OpenAiSelfHarmSignal: instantiate configured signal
OpenAiSelfHarmSignal->>getOpenAiScores: submit text or image input
getOpenAiScores->>OpenAI moderation API: request moderation score
OpenAI moderation API-->>getOpenAiScores: return category score
getOpenAiScores-->>OpenAiSelfHarmSignal: return numeric result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
4 tasks
julietshen
force-pushed
the
julietshen/mod-api-self-harm-subcategories
branch
2 times, most recently
from
May 21, 2026 12:13
da6a4ab to
e0e76a7
Compare
julietshen
force-pushed
the
julietshen/mod-api-self-harm-subcategories
branch
from
May 26, 2026 17:49
e0e76a7 to
345af20
Compare
Round out the omni-moderation self-harm coverage that the prior commit left lopsided. OpenAI scores both subcategories for text *and* image inputs; we had neither. New signal classes (4): - OpenAiSelfHarmIntentTextSignal / OpenAiSelfHarmIntentImageSignal - OpenAiSelfHarmInstructionsTextSignal / OpenAiSelfHarmInstructionsImageSignal Plus matching SignalType enum entries, integrationForSignalType cases, SignalArgsByType / RuntimeSignalArgsByType entries, IoC registration, and the two new category names in OpenAiModelName + OpenAiImageModelName. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same shape as the previous fix on PR #534: the four new self-harm subcategory signal types need to round-trip through both hand-maintained mirrors or the dashboard can't see them. - server/graphql/modules/signal.ts: add the four OPEN_AI_SELF_HARM_{INTENT,INSTRUCTIONS}_{TEXT,IMAGE}_MODEL types to the SDL enum. - client/src/models/signal.ts: add the same four to the OpenAi case in integrationForSignalType. - Regenerate codegen. The coverage test added in PR #534 catches this regression class on new SignalType additions; it now exercises 33 enum values (was 29). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Matches the docstring style applied to the base PR's image signals. Raises this PR's docstring coverage above CodeRabbit's 80% pre-merge threshold by documenting each of the four new signal classes (self-harm/intent text+image, self-harm/instructions text+image) and their run() methods. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the cleanup from PR #534: the 4 self-harm subcategory signal classes (2 text + 2 image) collapse to ~20-line factory calls instead of ~110-line class definitions each. - Adds `makeOpenAiTextModerationSignal` alongside the existing image factory introduced in #534. - Both factories now share a single internal `makeOpenAiModerationSignal` parameterized by input scalar + run impl, so the class body lives in one place (was duplicated twice in #534's first cut). - Factories moved into `openAiModerationSignalFactory.ts` — keeps `openAIModerationUtils.ts` under the 500-line lint limit (the second factory pushed it to 510) and organizes the file boundary cleanly: utils = lib functions, factory = class generator. The 4 self-harm subcategory signal files (text + image variants for `self-harm/intent` and `self-harm/instructions`) now just import the appropriate factory and pass a config object. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
julietshen
force-pushed
the
julietshen/mod-api-self-harm-subcategories
branch
from
May 29, 2026 22:49
345af20 to
cce7cbf
Compare
Contributor
|
Checklist:
|
juanmrad
approved these changes
Aug 4, 2026
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/services/signalsService/signals/third_party_signals/open_ai/moderation/openAiModerationSignalFactory.ts (1)
183-191: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMigrate the remaining OpenAI text moderation signals to the new factory.
The existing text subclasses still declare the shared moderation base getters by hand, while image signals and only the new self-harm text signals use
makeOpenAiImageModerationSignal/makeOpenAiTextModerationSignal. Use the wrappers for the remaining text signals to keep the moderation boilerplate in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/services/signalsService/signals/third_party_signals/open_ai/moderation/openAiModerationSignalFactory.ts` around lines 183 - 191, Update the remaining OpenAI text moderation signal subclasses to use makeOpenAiTextModerationSignal, removing their duplicated shared moderation base getters while preserving each signal’s specific configuration and behavior. Follow the existing implementation of makeOpenAiTextModerationSignal and the migrated self-harm text signals as the pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@server/services/signalsService/signals/third_party_signals/open_ai/moderation/openAiModerationSignalFactory.ts`:
- Around line 1-12: Add unit tests for both makeOpenAiImageModerationSignal and
makeOpenAiTextModerationSignal, covering their expected eligibleInputs values,
id.type matching the configured SignalType, and run forwarding modelName plus
the injected credential getter to the correct implementation. Use mocks or spies
for the underlying moderation implementations and verify each wrapper routes to
its corresponding implementation.
---
Nitpick comments:
In
`@server/services/signalsService/signals/third_party_signals/open_ai/moderation/openAiModerationSignalFactory.ts`:
- Around line 183-191: Update the remaining OpenAI text moderation signal
subclasses to use makeOpenAiTextModerationSignal, removing their duplicated
shared moderation base getters while preserving each signal’s specific
configuration and behavior. Follow the existing implementation of
makeOpenAiTextModerationSignal and the migrated self-harm text signals as the
pattern.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7843d854-41df-42e5-9ed0-8d580e561fa8
⛔ Files ignored due to path filters (2)
client/src/graphql/generated.tsis excluded by!**/generated.tsserver/graphql/generated.tsis excluded by!**/generated.ts
📒 Files selected for processing (15)
client/src/models/signal.tsserver/graphql/modules/signal.tsserver/services/signalsService/helpers/instantiateBuiltInSignals.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/OpenAiGraphicViolenceImageSignal.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/OpenAiSelfHarmImageSignal.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/OpenAiSelfHarmInstructionsImageSignal.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/OpenAiSelfHarmInstructionsTextSignal.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/OpenAiSelfHarmIntentImageSignal.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/OpenAiSelfHarmIntentTextSignal.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/OpenAiSexualImageSignal.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/OpenAiViolenceImageSignal.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/openAIModerationUtils.tsserver/services/signalsService/signals/third_party_signals/open_ai/moderation/openAiModerationSignalFactory.tsserver/services/signalsService/types/SignalArgsByType.tsserver/services/signalsService/types/SignalType.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- server/services/signalsService/types/SignalType.ts
- client/src/models/signal.ts
- server/graphql/modules/signal.ts
- server/services/signalsService/helpers/instantiateBuiltInSignals.ts
- server/services/signalsService/types/SignalArgsByType.ts
Comment on lines
+1
to
+12
| /** | ||
| * Factories for OpenAI moderation signals. Collapses the boilerplate that | ||
| * was duplicated across every per-category signal class — id, displayName, | ||
| * description, integration, pricing, languages, cost, etc. The two public | ||
| * exports (`makeOpenAiImageModerationSignal`, | ||
| * `makeOpenAiTextModerationSignal`) are thin wrappers around the private | ||
| * `makeOpenAiModerationSignal` so the class body lives in one place. | ||
| * | ||
| * The IoC container instantiates the returned class with | ||
| * `(credentials, scores)` like any other signal, preserving the existing | ||
| * registration pattern in `instantiateBuiltInSignals.ts`. | ||
| */ |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add tests for the shared factory.
This module is now the single construction point for every OpenAI moderation signal. A regression in eligibleInputs, outputType, id, or runImpl routing would affect all of them at once. The PR adds no tests.
Add a unit test that asserts, for both wrappers:
eligibleInputsreturns[ScalarTypes.IMAGE]formakeOpenAiImageModerationSignaland[ScalarTypes.STRING]formakeOpenAiTextModerationSignal.runforwards the configuredmodelNameand the injected credential getter to the correct implementation.id.typeequals the configuredSignalType.
Do you want me to generate that test file?
Based on learnings, and as per coding guidelines: "New behavior requires a test, and bug fixes require a regression test."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@server/services/signalsService/signals/third_party_signals/open_ai/moderation/openAiModerationSignalFactory.ts`
around lines 1 - 12, Add unit tests for both makeOpenAiImageModerationSignal and
makeOpenAiTextModerationSignal, covering their expected eligibleInputs values,
id.type matching the configured SignalType, and run forwarding modelName plus
the injected credential getter to the correct implementation. Use mocks or spies
for the underlying moderation implementations and verify each wrapper routes to
its corresponding implementation.
Source: Coding guidelines
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
3 participants
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.
Stacked on #534. Adds the two omni self-harm subcategories on both modalities so coverage is symmetric and matches OpenAI's spec.
Why this is separate
PR #534 resolves #518 by adding image moderation for the four omni categories that already had text counterparts (
violence,violence/graphic,self-harm,sexual). It deliberately stops there to keep the diff focused.But OpenAI's omni model also scores
self-harm/intentandself-harm/instructionsfor both text and images — and we had neither modality for those subcategories. Image-only would be lopsided; this PR adds the missing four classes so both modalities cover the full self-harm trio in OpenAI's spec.What's in
Four new signal classes following the existing pattern:
OpenAiSelfHarmIntentTextSignalOpenAiSelfHarmIntentImageSignalOpenAiSelfHarmInstructionsTextSignalOpenAiSelfHarmInstructionsImageSignalPlus the matching plumbing: two new category names in
OpenAiModelNameandOpenAiImageModelName, four newSignalTypeenum values, four entries each inSignalArgsByType/RuntimeSignalArgsByType, four cases inintegrationForSignalType, and four IoC registrations ininstantiateBuiltInSignals.Tests
The existing
openAIModerationUtils.test.tsfrom #534 still passes — no new test surface added, since these signals are pure delegations to the lib that PR #534 already tests.Reviewer checks:
(Optional) Rollout Plan
No schema change, no GraphQL change, no client change. Server starts registering 4 additional built-in signal types alongside the ones added in #534.
🤖 Generated with Claude Code
Summary by CodeRabbit