Skip to content

Commit c7a7426

Browse files
davidercruzclaude
andauthored
feat: respect blocked tags in briefing requests (#3629)
Co-authored-by: Claude Opus 4.6 <[email protected]>
1 parent 7147029 commit c7a7426

7 files changed

Lines changed: 42 additions & 17 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ The migration generator compares entities against the local database schema. Ens
9999
- Always consult the [Zod 4.x documentation](https://zod.dev) for the latest API
100100
- When possible, prefer Zod schemas over manual validation as they provide type safety, better error messages, and can be inferred to TypeScript types.
101101
- **Connect RPC handlers must return typed proto message classes** from `@dailydotdev/schema`, not plain objects. Use `new ResponseType({...})` instead of returning `{...}` directly. **This applies to mock/`isMockEnabled` returns too** — when mocking RPC transport, always use actual proto message class instances, not raw JSON objects.
102+
- **Never create wrapper types around `@dailydotdev/schema` classes** (e.g., `UserBriefingRequest & { extraField }`) — if a field exists in the proto, use it directly. If it doesn't exist yet, update the schema package first.
102103
```typescript
103104
// BAD: plain object
104105
return {
@@ -159,6 +160,7 @@ The migration generator compares entities against the local database schema. Ens
159160
- Mercurius integration testing for GraphQL endpoints
160161
- Avoid creating multiple overlapping tests for the same scenario; a single test per key scenario is preferred
161162
- When evaluating response objects (GraphQL, API), prefer `toEqual` and `toMatchObject` over multiple `expect().toBe()` lines
163+
- **Prefer strict schema assertions over `expect.objectContaining`** — when verifying proto message payloads (e.g., `UserBriefingRequest`), use `new SchemaClass({...})` to assert exact structure. `expect.objectContaining` weakens validation by ignoring extra fields, which defeats schema correctness checks.
162164
- Avoid redundant test assertions - if an assertion already verifies the value, don't add negative checks that are logically implied (e.g., if `expect(result).toBe('a')` passes, don't also check `expect(result).not.toBe('b')`)
163165
- When adding/removing persisted entity fields, update affected Jest snapshots in worker/integration tests (for example `toMatchSnapshot` payloads) as part of the same change to avoid CI drift.
164166
- **Typed worker tests**: Always use the generic type parameter with `expectSuccessfulTypedBackground<'topic-name'>()` for type safety. Use `toChangeObject()` to convert entities to the expected message payload format:

__tests__/workers/brief/userGenerateBrief.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ describe('userGenerateBrief worker', () => {
173173
frequency: BriefingType.Daily,
174174
model_name: BriefingModel.Default,
175175
allowed_tags: ['webdev', 'development'],
176+
blocked_tags: ['fullstack'],
176177
seniority_level: 'NOT_ENGINEER',
177178
});
178179

@@ -203,6 +204,7 @@ describe('userGenerateBrief worker', () => {
203204
frequency: BriefingType.Daily,
204205
modelName: BriefingModel.Default,
205206
allowedTags: ['webdev', 'development'],
207+
blockedTags: ['fullstack'],
206208
seniorityLevel: 'NOT_ENGINEER',
207209
}),
208210
postId,
@@ -355,6 +357,7 @@ describe('userGenerateBrief worker', () => {
355357
frequency: BriefingType.Daily,
356358
model_name: BriefingModel.Default,
357359
allowed_tags: ['webdev', 'development'],
360+
blocked_tags: ['fullstack'],
358361
seniority_level: 'NOT_ENGINEER',
359362
recent_briefing: {
360363
sections: [
@@ -400,6 +403,7 @@ describe('userGenerateBrief worker', () => {
400403
frequency: BriefingType.Daily,
401404
modelName: BriefingModel.Default,
402405
allowedTags: ['webdev', 'development'],
406+
blockedTags: ['fullstack'],
403407
seniorityLevel: 'NOT_ENGINEER',
404408
recentBriefing: {
405409
sections: [
@@ -408,7 +412,7 @@ describe('userGenerateBrief worker', () => {
408412
items: [
409413
{
410414
title: 'OpenAI gets a DoD contract, Microsoft gets salty',
411-
body: `OpenAI landed a $200 million contract with the US Department of Defense for AI tools, marking its first direct federal government partnership. This move, reported by The Verge and TechCrunch, signals a shift from OpenAI’s previous stance on military use. It also puts them in direct competition with Microsoft, their main investor, who previously handled government AI contracts through Azure. The tension is real, with OpenAI reportedly considering an antitrust complaint against Microsoft to loosen their grip.`,
415+
body: `OpenAI landed a $200 million contract with the US Department of Defense for AI tools, marking its first direct federal government partnership. This move, reported by The Verge and TechCrunch, signals a shift from OpenAI\u2019s previous stance on military use. It also puts them in direct competition with Microsoft, their main investor, who previously handled government AI contracts through Azure. The tension is real, with OpenAI reportedly considering an antitrust complaint against Microsoft to loosen their grip.`,
412416
},
413417
],
414418
},
@@ -525,6 +529,7 @@ describe('userGenerateBrief worker', () => {
525529
frequency: BriefingType.Daily,
526530
model_name: BriefingModel.Default,
527531
allowed_tags: ['webdev', 'development'],
532+
blocked_tags: ['fullstack'],
528533
seniority_level: 'NOT_ENGINEER',
529534
recent_briefing: {
530535
sections: [
@@ -702,6 +707,7 @@ describe('userGenerateBrief worker', () => {
702707
frequency: BriefingType.Daily,
703708
model_name: BriefingModel.Default,
704709
allowed_tags: ['webdev', 'development'],
710+
blocked_tags: ['fullstack'],
705711
seniority_level: 'NOT_ENGINEER',
706712
});
707713

@@ -733,6 +739,7 @@ describe('userGenerateBrief worker', () => {
733739
frequency: BriefingType.Daily,
734740
modelName: BriefingModel.Default,
735741
allowedTags: ['webdev', 'development'],
742+
blockedTags: ['fullstack'],
736743
seniorityLevel: 'NOT_ENGINEER',
737744
}),
738745
postId,

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
"@connectrpc/connect-fastify": "^1.6.1",
3838
"@connectrpc/connect-node": "^1.6.1",
3939
"@dailydotdev/graphql-redis-subscriptions": "^2.4.3",
40-
"@dailydotdev/schema": "0.2.72",
40+
"@dailydotdev/schema": "0.2.73",
4141
"@dailydotdev/ts-ioredis-pool": "^1.0.2",
4242
"@fastify/cookie": "^11.0.2",
4343
"@fastify/cors": "^11.2.0",

pnpm-lock.yaml

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/common/brief.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,14 @@ export const getUserConfigForBriefingRequest = async ({
4646
}: {
4747
con: EntityManager;
4848
userId: string;
49-
}): Promise<Pick<UserBriefingRequest, 'allowedTags' | 'seniorityLevel'>> => {
49+
}): Promise<
50+
Pick<UserBriefingRequest, 'allowedTags' | 'blockedTags' | 'seniorityLevel'>
51+
> => {
5052
if (!userId) {
5153
throw new Error('User id is required');
5254
}
5355

54-
const [user, keywords] = await Promise.all([
56+
const [user, keywords, blockedKeywords] = await Promise.all([
5557
con.getRepository(User).findOneOrFail({
5658
select: ['id', 'experienceLevel'],
5759
where: {
@@ -69,10 +71,19 @@ export const getUserConfigForBriefingRequest = async ({
6971
]),
7072
},
7173
}),
74+
con.getRepository(ContentPreferenceKeyword).find({
75+
select: ['keywordId'],
76+
where: {
77+
userId: userId,
78+
feedId: userId,
79+
status: ContentPreferenceStatus.Blocked,
80+
},
81+
}),
7282
]);
7383

7484
return {
7585
allowedTags: keywords.map((item) => item.keywordId),
86+
blockedTags: blockedKeywords.map((item) => item.keywordId),
7687
seniorityLevel: user.experienceLevel ?? undefined,
7788
};
7889
};

src/integrations/feed/clients.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,17 @@ export class FeedClient implements IFeedClient, IGarmrClient {
7878
};
7979
}
8080

81-
async getUserBrief({
82-
userId,
83-
frequency,
84-
modelName = BriefingModel.Default,
85-
allowedTags,
86-
seniorityLevel,
87-
recentBriefing,
88-
}: UserBriefingRequest): Promise<Briefing> {
81+
async getUserBrief(request: UserBriefingRequest): Promise<Briefing> {
82+
const {
83+
userId,
84+
frequency,
85+
modelName = BriefingModel.Default,
86+
allowedTags,
87+
blockedTags,
88+
seniorityLevel,
89+
recentBriefing,
90+
} = request;
91+
8992
const result = await this.garmr.execute(() => {
9093
return fetchParse<JsonValue>(`${this.url}/api/user/briefing`, {
9194
...this.fetchOptions,
@@ -95,6 +98,7 @@ export class FeedClient implements IFeedClient, IGarmrClient {
9598
frequency,
9699
model_name: modelName,
97100
allowed_tags: allowedTags,
101+
blocked_tags: blockedTags,
98102
seniority_level: seniorityLevel,
99103
recent_briefing: recentBriefing
100104
? {

src/workers/brief/userGenerateBrief.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ export const userGenerateBriefWorker: TypedWorker<'api.v1.brief-generate'> = {
9595
);
9696

9797
briefRequest.allowedTags = userConfig.allowedTags;
98+
briefRequest.blockedTags = userConfig.blockedTags;
9899
briefRequest.seniorityLevel = userConfig.seniorityLevel;
99100

100101
const lastBriefPost = await queryReadReplica<Pick<

0 commit comments

Comments
 (0)