feat: web search and notification adjustment

[capJavert]

No description provided.

[capJavert]

@claude review

[claude]

Claude finished @capJavert's task in 4m 15s —— View job

---

Review: PR #4022 — web search & notification adjustment

• Read the changed files
• Analyze for bugs, security, performance, style
• Post review feedback

Overall this is a clean, well-tested addition. The discover_external agent tool, the origin column, per-run notification dedup, and the tool-gating are all coherent and the test coverage (discoverExternalUrls, runInterestAgent tool gating, notification dedup) is solid. A few things worth addressing before merge.

---

🟠 1. Over-counting on .orIgnore() — inflates count and burns the daily cap

src/common/interest/runInterestAgent.ts (discovery loop, ~L505–521)

The discovery finding is inserted with .orIgnore() on the (interestId, postId) unique index, but the code unconditionally does:

addedPostIds.add(postId);
state.findingsAdded += 1;
added += 1;

When the finding already exists (post already discovered/added for this interest), the insert is a no-op yet still counts. Consequences:

• state.findingsAdded → drives newFindings/count in userInterestRun.ts → the interest_content_batch notification claims more new posts than were actually added.
• added counts toward remaining (the per-day discovery cap), so re-surfacing existing content wastes the budget.

Consider checking the insert result (.execute() → result.identifiers/raw length) and only incrementing on an actual insert. Note the search path uses .orUpdate(...) so it doesn't hit this, but there the finding is genuinely (re)written. Fix this →

🟠 2. Discovery bypasses the public-visibility guard that the feed path enforces

src/common/interest/runInterestAgent.ts (~L484–497)

The add_to_feed tool only accepts posts that are private: false, deleted: false, showOnFeed: true (returns not_public otherwise). The discovery path skips that check and inserts a finding pointing at a post created by createExternalLink, which produces a post with private: true, sourceId: UNKNOWN_SOURCE, and visible = !!title (a title-less candidate → visible: false). Content is only scraped asynchronously via notifyContentRequested, and createExternalLink never clears private.

Since findings surface via the interestFindings resolver (team tool, no visibility filter) and the /agent/{id} page, this may be intentional — but please confirm discovered posts actually render for the end user, especially before scraping completes. If any downstream delivery filters on private = false / visible = true (as interestPosts filters by the interest's sourceId, which discovered UNKNOWN_SOURCE posts will never match), freshly discovered content won't show. There's also no code-level guard preventing the model from returning daily.dev URLs — only the system prompt asks it not to.

🟡 3. URLs are not standardized before dedup/insert

src/common/interest/runInterestAgent.ts (~L486, L495)

const existing = await getExistingPost(con, { url: candidate.url, canonicalUrl: candidate.url });
...
await createExternalLink({ con, args: { ...url: candidate.url, canonicalUrl: candidate.url, originalUrl: candidate.url } });

Other ingestion paths run the URL through standardizeURL(...) before dedup so tracking params / trailing slashes collapse. Passing the raw model-returned URL as both url and canonicalUrl risks missing an existing post and creating a duplicate external-link post.

🟡 4. Sequential awaits inside the candidate loop

Each candidate does getExistingPost → createExternalLink (its own transaction) → InterestFinding insert serially. Bounded to ≤10 (DEFAULT_MAX_DISCOVERIES_PER_DAY/max), so acceptable, but flagging per the repo's N+1 guidance.

🟢 Nits

• src/common/interest/discoverExternalUrls.ts: let response; evolves to any until assigned — annotate let response: AnthropicResponse; for clarity.
• max_tokens: 4096 with web-search overhead could truncate a large JSON reply; extractJsonArray would then fail to parse and return [] silently. Fine as a fallback, just noting the failure is invisible beyond the warn log.
• extractJsonArray uses first [ / last ], which could capture a bracketed substring embedded in prose; low risk given the strict JSON-only instruction and the [] fallback.
• Migration AddInterestFindingOrigin uses ADD COLUMN without IF NOT EXISTS. Not required for columns, but the repo convention leans idempotent; existing rows correctly backfill to 'search'.

✅ Nice

• Per-run notification fix (uniqueKey(ctx.dedupKey ?? ctx.interest.id) + dedupKey: ${interestId}:${runAt}) is consistent across both NotificationV2.uniqueKey and the UserNotification key, and the new tests lock in one-notification-per-run with in-run dedup.
• Tool gating (web source + feed output required for discover_external) is correctly reserved and well covered.
• discoverExternalUrls fails soft (returns [], never throws) — good, keeps a flaky web search from failing the whole run.
  · feat/interest-agent-discovery-context

[pulumi]

🍹 The Update (preview) for dailydotdev/api/prod (at a1c4462) was successful.

Resource Changes

    Name                                                       Type                           Operation
~   vpc-native-rotate-daily-quests-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-achievements-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-post-lifecycle-state-clickhouse-cron            kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-worker-job-deployment                           kubernetes:apps/v1:Deployment  update
~   vpc-native-check-analytics-report-cron                     kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-stale-user-transactions-cron              kubernetes:batch/v1:CronJob    update
-   vpc-native-api-db-migration-903c38ae                       kubernetes:batch/v1:Job        delete
~   vpc-native-post-analytics-history-day-clickhouse-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-generate-search-invites-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-ws-deployment                                   kubernetes:apps/v1:Deployment  update
~   vpc-native-private-deployment                              kubernetes:apps/v1:Deployment  update
~   vpc-native-update-tag-materialized-views-cron              kubernetes:batch/v1:CronJob    update
-   vpc-native-api-clickhouse-migration-903c38ae               kubernetes:batch/v1:Job        delete
~   vpc-native-update-trending-cron                            kubernetes:batch/v1:CronJob    update
+   vpc-native-api-clickhouse-migration-5419c545               kubernetes:batch/v1:Job        create
~   vpc-native-hourly-notification-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-opportunities-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-gifted-plus-cron                          kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-images-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-channel-highlights-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-expired-better-auth-sessions-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-old-notifications-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-clickhouse-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-update-source-public-threshold-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-users-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment                             kubernetes:apps/v1:Deployment  update
+   vpc-native-api-db-migration-5419c545                       kubernetes:batch/v1:Job        create
~   vpc-native-user-profile-analytics-history-clickhouse-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-user-companies-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-bg-deployment                                   kubernetes:apps/v1:Deployment  update
~   vpc-native-generic-referral-reminder-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-updated-sync-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-deployment                  kubernetes:apps/v1:Deployment  update
~   vpc-native-rotate-weekly-quests-cron                       kubernetes:batch/v1:CronJob    update
~   vpc-native-update-achievement-rarity-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                               kubernetes:batch/v1:CronJob    update
~   vpc-native-subscription-anniversary-achievements-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-channel-digests-cron                            kubernetes:batch/v1:CronJob    update
~   vpc-native-materialize-monthly-best-post-archives-cron     kubernetes:batch/v1:CronJob    update
~   vpc-native-interest-scheduled-run-cron                     kubernetes:batch/v1:CronJob    update
... and 16 other changes

[capJavert]

@claude review

[claude]

Claude finished @capJavert's task in 3m 25s —— View job

---

Review: PR #4022 — web search & notification adjustment

• Read the changed files
• Analyze for bugs, security, performance, style
• Post review feedback

Re-reviewed at f7c9608. The feat: review feedback commit resolved the main items from the prior pass — nice work:

• Over-counting fixed — discover_external now checks insertResult.raw?.length and only bumps state.findingsAdded/added/addedPostIds on a real insert. (This works because TypeORM emits RETURNING for @CreateDateColumn, so .orIgnore() yields an empty raw on conflict — correct.)
• URL standardization added — candidates run through standardizeURL(...) with in-call seenCanonical dedup before getExistingPost/createExternalLink.
• daily.dev guard is now code-level — isDailyDevUrl in discoverExternalUrls, backed by a test.
• Batching — sequential awaits replaced with blockingBatchRunner + Promise.all.
• let response: AnthropicResponse; annotation applied.

A few things remain — nothing blocking, mostly one product confirm and a test gap.

---

🟠 1. Confirm discovered posts actually render for the end user

src/common/interest/runInterestAgent.ts (discovery path) + src/schema/interests.ts:205

This is the one carry-over worth nailing down before merge. add_to_feed only accepts posts that are private:false, deleted:false, showOnFeed:true. The discovery path instead points a finding at a createExternalLink post, which is created with private:true, sourceId: UNKNOWN_SOURCE, and visible = !!title (a title-less candidate → visible:false, content scraped async via notifyContentRequested).

Findings surface through interestFindings (GraphORM, no visibility filter, but ensureTeamMember-gated) and the /agent/{id} page — so for team members they'll load. Two things to verify:

• The end-user delivery path (not the team tool) renders private:true / not-yet-scraped posts. If any downstream feed filters on private=false/visible=true, discovered content won't show until scraping completes — or at all.
• interestPosts (src/schema/interests.ts:237) filters by the interest's sourceId, which discovered UNKNOWN_SOURCE posts will never match. That's expected (that resolver is for the generated summary post), just calling it out so it's intentional.

If this is by design, a one-line comment on the discovery insert noting the private/async-scrape behavior would save the next reader the trace.

🟡 2. No test covers the discover_external execute path

__tests__/common/interest/runInterestAgent.ts

Coverage is good for discoverExternalUrls (parsing/dedup/daily.dev/fail-soft) and tool gating, but the discover_external tool body itself — the piece that had the counting bug — isn't tested. Given it was the highest-risk change, a test that asserts (a) a duplicate (interestId, postId) insert does not inflate findingsAdded/added, (b) below-fomoThreshold candidates are skipped, and (c) the daily cap short-circuits, would lock in the fix and prevent regression. Add test →

🟢 Nits

• Race window: getExistingPost → createExternalLink isn't atomic, so two concurrent runs discovering the same URL could each create an external-link post. Bounded and pre-existing across ingestion paths, but the concurrent Promise.all batch widens it slightly. Low risk given ≤10 candidates/call and the finding .orIgnore() on (interestId, postId).
• discoverCalls += 1 runs before the budget check, so an over-budget call still increments the counter. Harmless (already rejected), just slightly off semantically.
• max_tokens: 4096 with web-search overhead could truncate a large JSON reply → extractJsonArray fails and returns [] silently (only a warn). Fine as a soft-fail, noted for observability.
• Migration AddInterestFindingOrigin correctly uses IF NOT EXISTS and backfills existing rows to 'search' — 👍. Note the new InterestFindingOrigin.Live enum value is written by postVisibleInterestMatch, so no backfill needed for it.

✅ Nice

• Per-run notification dedup (uniqueKey(ctx.dedupKey ?? ctx.interest.id) + dedupKey: ${interestId}:${runAt}) is consistent across NotificationV2.uniqueKey and the UserNotification key, with tests locking in one-notification-per-run and in-run dedup.
• Tool gating (web source and feed output required for discover_external, github reserved for later) is correct and well covered.
• discoverExternalUrls fails soft (returns [], never throws) — keeps a flaky web search from failing the whole run.
• Origin column threading through search/live/discovery paths is clean.
  · feat/interest-agent-discovery-context