Skip to content

feat(monitors): upgrade monitors with rules, highlights, and editing#2398

Open
FayezBast wants to merge 2 commits intomainfrom
feat/escalation-replay
Open

feat(monitors): upgrade monitors with rules, highlights, and editing#2398
FayezBast wants to merge 2 commits intomainfrom
feat/escalation-replay

Conversation

@FayezBast
Copy link
Copy Markdown
Collaborator

Summary

  • upgrade My Monitors from basic keyword watchlists to richer monitor rules
  • add in-panel monitor matches and highlight matched stories across news panels
  • add edit-in-place for saved monitors
  • add free/pro gating for advanced monitor options
  • add monitor migration + test coverage for the new monitor flow

Type of change

  • New feature

Affected areas

  • News panels / RSS feeds
  • Config / Settings
  • Other: Monitors

Checklist

  • No API keys or secrets committed
  • TypeScript compiles without errors (npm run typecheck)

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
worldmonitor Ready Ready Preview, Comment Mar 31, 2026 1:08am

Request Review

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 28, 2026

Greptile Summary

This PR upgrades the My Monitors feature from simple keyword watchlists to a richer rule-based system with include/exclude keywords, match modes (any/all), multi-source targeting (news, breaking alerts, security advisories, cross-source signals), edit-in-place, and free/pro feature gating. A new src/services/monitors.ts centralises all matching, normalisation, and highlight logic; MonitorPanel is extensively rewritten; and startup migration via normalizeMonitors ensures backward-compatibility for existing stored monitors. Test coverage is added for the core service functions.

Key findings:

  • P1 — Silent monitor blackout in prepareMonitorsForRuntime: When a monitor is saved with only pro-tier sources and the user is on the free tier, Array.filter() returns []; because [] is not null/undefined, the ?? [...FREE_MONITOR_SOURCES] fallback never fires, leaving sources: []. The monitor silently produces zero matches with no user feedback.
  • P1 — ctx.newsByCategory not updated with highlights: updateMonitorResults writes highlighted items back to ctx.allNews but only renders (not stores) highlights for newsByCategory. Any re-render from stored category data will show stale, un-highlighted items.
  • P1 — item.link included in keyword search haystack: News article URLs and source names are included in the match haystack, causing false positives (e.g. a URL path containing a topic slug from an unrelated article triggering a monitor match).
  • P2 — Cookie gate uses substring search: document.cookie.includes(...) can be satisfied by a cookie whose name merely contains the target string, inadvertently granting pro access.
  • P2 — Free-limit status shown to pro users: applyComposerAccessState() unconditionally renders the "Free: up to N monitors" status message even for users with pro access.

Confidence Score: 4/5

Hold for the three P1 issues — the empty-sources silent failure, ctx.newsByCategory inconsistency, and URL-in-haystack false positives — before merging.

Three P1 findings exist: (1) a real data-correctness bug where a monitor with only pro-tier sources silently produces zero matches after a downgrade, (2) a state inconsistency that causes news highlights to disappear on re-render from stored category data, and (3) item.link in the keyword haystack generating false positive matches from URL slugs. All three affect the primary monitor-matching user path in ways users will notice. The rest of the feature is well-structured, has good migration logic, and is covered by tests.

src/services/monitors.ts (prepareMonitorsForRuntime empty-sources fallback, matchesKeyword haystack composition) and src/app/data-loader.ts (ctx.newsByCategory not updated).

Important Files Changed

Filename Overview
src/services/monitors.ts New core service: monitor normalization, match evaluation, pro-gating, and news highlighting — has a silent empty-sources bug in prepareMonitorsForRuntime and URL-in-haystack false-positive issue.
src/app/data-loader.ts updateMonitorResults now applies highlights and feeds the monitor panel — but ctx.newsByCategory is not updated, creating a state inconsistency with ctx.allNews.
src/components/MonitorPanel.ts Major rewrite of the monitor UI: adds rich rule composer, edit-in-place, source toggles, and live match cards — event listener properly cleaned up in destroy(); minor: pro users see free-limit status message.
src/types/index.ts Adds MonitorMatchMode, MonitorSourceKind types and extends Monitor with includeKeywords, excludeKeywords, matchMode, sources, radiusKm, createdAt, updatedAt — all optional for backward compatibility.
tests/monitors.test.mts New test file covering normalization, pro-gating, cross-source matching, exclude keywords, prefix matching, and highlight application — good coverage of the happy paths.

Sequence Diagram

sequenceDiagram
    participant App as App.ts
    participant DL as DataLoaderManager
    participant MS as monitors.ts
    participant MP as MonitorPanel
    participant Ctx as AppContext

    App->>MS: normalizeMonitors(loadFromStorage)
    App->>Ctx: ctx.monitors = normalized

    note over DL: On news refresh / monitor change
    DL->>MS: applyMonitorHighlightsToNews(ctx.monitors, ctx.allNews)
    MS-->>DL: highlightedAllNews
    DL->>Ctx: ctx.allNews = highlightedAllNews

    loop each category
        DL->>MS: applyMonitorHighlightsToNews(monitors, items)
        MS-->>DL: highlightedItems
        DL->>DL: renderNewsForCategory(category, highlightedItems)
    end

    DL->>MP: renderResults({news, advisories, crossSourceSignals})
    MP->>MS: evaluateMonitorMatches(monitors, feed)
    MS-->>MP: MonitorMatch[]
    MP->>MP: refreshResults()

    note over MP: On breaking-news event
    document-->>MP: wm:breaking-news CustomEvent
    MP->>MP: breakingAlerts.unshift(detail), slice(0,50)
    MP->>MS: evaluateMonitorMatches(monitors, feed + breakingAlerts)
    MS-->>MP: MonitorMatch[]
    MP->>MP: refreshResults()
Loading

Comments Outside Diff (4)

  1. src/services/monitors.ts, line 865-877 (link)

    P1 Silent empty-sources when pro-only monitor used by free user

    When a monitor is saved with only pro sources (e.g. sources: ['advisories', 'cross-source']) and the user is a free tier, monitor.sources?.filter(...) returns an empty array []. Because [] is not null/undefined, the nullish-coalescing fallback ?? [...FREE_MONITOR_SOURCES] never triggers and the stored monitor ends up with sources: []. Every subsequent call to evaluateMonitorMatches skips all source branches, producing zero matches silently with no feedback to the user.

    This can happen in production today if a user creates a monitor while on a pro trial, then their key expires.

    // current — ?? never fires when filter returns []
    sources: monitor.sources?.filter((source) => FREE_MONITOR_SOURCES.includes(source)) ?? [...FREE_MONITOR_SOURCES],
    
    // fix — treat an empty filtered result the same as undefined
    const freeSources = monitor.sources?.filter((s) => FREE_MONITOR_SOURCES.includes(s)) ?? [];
    sources: freeSources.length > 0 ? freeSources : [...FREE_MONITOR_SOURCES],
  2. src/app/data-loader.ts, line 66-73 (link)

    P1 ctx.newsByCategory not updated with highlights

    this.ctx.allNews is replaced with the highlighted copy (line 64), but this.ctx.newsByCategory is never updated — only the rendered output of each category panel is refreshed. Any subsequent code path that re-renders a category panel from ctx.newsByCategory (e.g. a panel layout change, resize, or tab-switch) will render un-highlighted items, while ctx.allNews still carries the highlighted version. This creates a persistent state inconsistency.

    Consider either updating ctx.newsByCategory[category] alongside the render call, or keeping highlights purely in the render layer without modifying stored context state.

  3. src/services/monitors.ts, line 975-983 (link)

    P1 item.link in keyword search haystack causes false positives

    The news haystack now includes item.link (the article URL). URLs frequently contain category slugs, author names, or related-topic slugs that do not reflect the article's actual content. For example, an article at https://reuters.com/world/china/brazil-agriculture-update/ would match a china monitor even if the article is entirely about Brazil.

    item.source has the same problem — a monitor for "reuters" would match every Reuters article regardless of content.

    Consider restricting the haystack to item.title, item.locationName, and the optional description/summary, which were the semantically meaningful fields.

    const haystack = [
      item.title,
      item.locationName,
      extraDescription,
    ].filter(Boolean).join(' ').toLowerCase();
  4. src/services/monitors.ts, line 1121-1134 (link)

    P2 Substring cookie check can produce false pro-gate match

    The cookie.includes(name) approach runs a plain substring search over the entire document.cookie string. A cookie whose name embeds the target string as a substring (e.g. a cookie prefixed with fake-) would still satisfy the check and unintentionally grant pro access. Consider splitting on '; ' and testing whether a segment starts with the exact key name followed by =, so only an exact-name match elevates access.

Reviews (1): Last reviewed commit: "feat(monitors): upgrade monitors with ru..." | Re-trigger Greptile

Comment on lines +313 to +329
this.setComposerStatus(t('components.monitor.freeLimit', { count: String(FREE_MONITOR_LIMIT) }), 'info');
}

private resetComposer(): void {
this.editingMonitorId = null;
if (this.nameInput) this.nameInput.value = '';
if (this.includeInput) this.includeInput.value = '';
if (this.excludeInput) this.excludeInput.value = '';
if (this.modeSelect) this.modeSelect.value = 'any';
for (const [source, input] of this.sourceInputs) {
input.checked = source === 'news' || source === 'breaking';
}
if (this.addBtn) this.addBtn.textContent = t('components.monitor.add');
if (this.cancelBtn) this.cancelBtn.style.display = 'none';
}

private renderMonitorCard(monitor: Monitor): HTMLElement {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Free-limit status message shown to pro users

applyComposerAccessState() unconditionally calls setComposerStatus(t('components.monitor.freeLimit', ...)) at the end, regardless of whether the user has pro access. A pro user will see "Free: up to 3 monitors…" even though they have no limit and can use advanced features. The message should be suppressed (or replaced with a pro-context message) when proAccess === true.

if (!proAccess) {
  this.setComposerStatus(
    t('components.monitor.freeLimit', { count: String(FREE_MONITOR_LIMIT) }),
    'info',
  );
}

@FayezBast
Copy link
Copy Markdown
Collaborator Author

like If u liked this idea I could work on it just waiting to know if I close or make the fixes. @koala73

@SebastienMelki
Copy link
Copy Markdown
Collaborator

@FayezBast — this is a valuable upgrade and we'd like to see it move forward. Three P1s from Greptile need fixing:

  1. Silent empty-sources (monitors.ts:865-877): Pro→free downgrade returns [] which doesn't trigger the ?? fallback. Fix: check .length and fall back to FREE_MONITOR_SOURCES if empty.
  2. newsByCategory not updated with highlights (data-loader.ts:66-73): allNews gets highlighted copy but newsByCategory doesn't. Re-renders from category data show un-highlighted items.
  3. item.link in keyword search haystack (monitors.ts:975-983): URLs contain category slugs causing false positive matches. Restrict haystack to title, locationName, and description.

Also: cookie.includes(name) should split on '; ' and check for exact key name match.

Once those are fixed this is ready to merge. Good work on the test coverage and migration logic!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants