Skip to content

Multiple UI/UX fixes -feb#2997

Open
mpcgrid wants to merge 8 commits intomainfrom
fix/feb-miscellaneous-fixes
Open

Multiple UI/UX fixes -feb#2997
mpcgrid wants to merge 8 commits intomainfrom
fix/feb-miscellaneous-fixes

Conversation

@mpcgrid
Copy link
Collaborator

@mpcgrid mpcgrid commented Feb 4, 2026

✅ Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • I ran and tested the code works

Testing

Manually tested each implementation.


Changelog

  • Updated Logs Page with the new implementation in time filter component

  • In TRQL editor users can now click on empty/blank spaces in the editor and the cursor will appear

  • Added CMD + / for line commenting in TRQL

  • Activated proper undo/redo functionality in CodeMirror (TRQL editor)

  • Added a check for new logs button, previously once the user got to the end of the logs he could not check for newer logs

  • Added showing MS in logs page Dates

  • Removed LOG_INFO internal logs, they are available with Admin Debug flag

  • Added support for correct timezone render on server side.

  • Increased CLICKHOUSE_LOGS_LIST_MAX_MEMORY_USAGE to 1GB

  • Changed Previous run/ Next run to J/K, consistent with previous/next page in Runs list

* Updated Logs Page with the new implementation in time filter component

* In TRQL editor users can now click on empty/blank spaces in the editor and the cursor will appear

* Added CMD + / for line commenting in TRQL

* Activated propper undo/redo functionality in CodeMirror (TRQL editor)

* Added a check fo new logs button, previously once the user got to the end of the logs he could not check for newer logs

* Added showing MS in logs page Dates

* Removed LOG_INFO internal logs, they are available with Admin Debug flag

* Added support for correct timezone render on server side.

* Increased CLICKHOUSE_LOGS_LIST_MAX_MEMORY_USAGE to 1GB
@changeset-bot
Copy link

changeset-bot bot commented Feb 4, 2026

⚠️ No Changeset found

Latest commit: 7b26f8d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 4, 2026

Walkthrough

This pull request introduces a user timezone preference system, enhances the code editor with line comment toggling and improved keyboard shortcuts, updates datetime components to respect user timezone settings, refines logs filtering and display, and adjusts keyboard navigation shortcuts. The timezone system includes a new TimezoneSetter component that syncs the browser timezone with server-stored preferences, a resource route for handling timezone updates, and helper functions for preference management. Editor improvements add Cmd/Ctrl-/ shortcuts for line comments and explicit undo/redo keybindings. Logs UI changes propagate retention limits through components, introduce a "check-for-new" flow to prepend recent logs, and switch to more accurate datetime rendering. Additional changes include a ClickHouse memory configuration increase and refinements to logs filtering logic.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.64% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Multiple UI/UX fixes -feb' is vague and uses non-descriptive terms like 'fixes' and 'misc' that don't convey specific information about the changeset. Replace the generic title with a more specific description of the primary change, e.g., 'Add timezone support and improve Logs Page UX' or identify the most significant feature.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description follows the template structure with all required sections (Checklist, Testing, Changelog) completed with meaningful content detailing the changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/feb-miscellaneous-fixes

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
apps/webapp/app/components/primitives/DateTime.tsx (1)

83-95: ⚠️ Potential issue | 🟡 Minor

Timezone label should match the timezone used for formatting.

When timeZone is undefined, the formatter uses userTimeZone but the label still shows UTC.

✅ Suggested fix
   const locales = useLocales();
   const userTimeZone = useUserTimeZone();
+  const displayTimeZone = timeZone ?? userTimeZone;

   const formattedDateTime = (
     <span suppressHydrationWarning>
       {formatDateTime(
         realDate,
-        timeZone ?? userTimeZone,
+        displayTimeZone,
         locales,
         includeSeconds,
         includeTime,
         includeDate,
         hour12
       ).replace(/\s/g, String.fromCharCode(32))}
-      {showTimezone ? ` (${timeZone ?? "UTC"})` : null}
+      {showTimezone ? ` (${displayTimeZone})` : null}
     </span>
   );
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (1)

369-420: ⚠️ Potential issue | 🟠 Major

Reset the check-for-new flag before loading more logs.

If a check-for-new request fails or returns no data, isCheckingForNewRef stays true and the next “load more” will prepend logs and skip cursor updates, effectively breaking pagination.

🛠️ Suggested fix
   const handleLoadMore = useCallback(() => {
     if (loadMoreUrl && fetcher.state === "idle") {
+      isCheckingForNewRef.current = false;
       // Store the current filter state before loading
       fetcherFilterStateRef.current = location.search;
       fetcher.load(loadMoreUrl);
     }
   }, [loadMoreUrl, fetcher, location.search]);

Also applies to: 474-484

🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/primitives/DateTime.tsx`:
- Around line 104-108: The "Local" tooltip's UTC offset is being derived from
the browser locale while the displayed time is formatted with userTimeZone
(variables: realDate, timeZone, userTimeZone, locales passed into
TooltipContent), causing mismatched offsets; update the offset computation
inside TooltipContent (or the helper it uses) to derive the offset from the ISO
string for the formatted local time (e.g., build the zoned date/time in
userTimeZone and extract the offset from its ISO representation) so the
displayed offset always matches the formatted local time; apply the same change
to the other occurrences noted around lines 385-393 and 442-446.

In `@apps/webapp/app/routes/resources.timezone.ts`:
- Around line 8-29: Wrap the call to request.json() in a try/catch inside the
action function and return a 400 JSON response when parsing fails; then validate
result.data.timezone against the schema and also against
Intl.supportedValuesOf("timeZone") (or a cached list) before calling
setTimezonePreference, returning a 400 with an "Invalid timezone" error if the
timezone is not in the supported list; ensure you still call
uiPreferencesStorage.commitSession(session) only after successful validation and
session creation.
🧹 Nitpick comments (3)
apps/webapp/app/components/code/CodeBlock.tsx (1)

450-476: Broaden containerRef to accept callback refs.

React.RefObject only allows object refs; it rejects callback refs. Using React.Ref<HTMLDivElement> is the standard pattern for flexible ref props that accept both object and callback refs.

Suggested change
type HighlightCodeProps = {
  theme: PrismTheme;
  code: string;
  language: Language;
  showLineNumbers: boolean;
  highlightLines?: number[];
  maxLineWidth?: number;
  className?: string;
  preClassName?: string;
  isWrapped: boolean;
  searchTerm?: string;
-  containerRef?: React.RefObject<HTMLDivElement>;
+  containerRef?: React.Ref<HTMLDivElement>;
};
apps/webapp/app/components/primitives/DateTime.tsx (1)

43-53: Consider guarding against invalid stored timezones.

If the stored preference is unsupported or corrupted, formatting can throw and break rendering. A small fallback keeps this resilient.

🔧 Suggested guard
 export function useUserTimeZone(): string {
   const rootData = useRouteLoaderData("root") as { timezone?: string } | undefined;
   const localTimeZone = useLocalTimeZone();
   // Use stored timezone from cookie, or fall back to browser's local timezone
-  return rootData?.timezone && rootData.timezone !== "UTC" ? rootData.timezone : localTimeZone;
+  const preferred =
+    rootData?.timezone && rootData.timezone !== "UTC" ? rootData.timezone : localTimeZone;
+  try {
+    new Intl.DateTimeFormat("en-US", { timeZone: preferred }).format(new Date());
+    return preferred;
+  } catch {
+    return localTimeZone;
+  }
 }
apps/webapp/app/components/logs/LogsTable.tsx (1)

217-230: Disable “Check for new logs” while a request is in-flight.

Prevents repeated clicks and gives clearer feedback.

♻️ Suggested tweak
             {onCheckForMore && (
               <Button
                 LeadingIcon={ArrowPathIcon}
                 variant="tertiary/small"
                 onClick={onCheckForMore}
+                disabled={isLoadingMore}
               >
-                Check for new logs
+                {isLoadingMore ? "Checking…" : "Check for new logs"}
               </Button>
             )}
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a45f56 and 03dde20.

📒 Files selected for processing (15)
  • apps/webapp/app/components/TimezoneSetter.tsx
  • apps/webapp/app/components/code/CodeBlock.tsx
  • apps/webapp/app/components/code/TSQLEditor.tsx
  • apps/webapp/app/components/code/codeMirrorSetup.ts
  • apps/webapp/app/components/logs/LogDetailView.tsx
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/components/primitives/DateTime.tsx
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
  • apps/webapp/app/root.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
  • apps/webapp/app/routes/resources.timezone.ts
  • apps/webapp/app/services/preferences/uiPreferences.server.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

**/*.{ts,tsx}: Always import tasks from @trigger.dev/sdk, never use @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Every Trigger.dev task must be exported and have a unique id property with no timeouts in the run function

Files:

  • apps/webapp/app/routes/resources.timezone.ts
  • apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/components/logs/LogDetailView.tsx
  • apps/webapp/app/components/code/codeMirrorSetup.ts
  • apps/webapp/app/components/primitives/DateTime.tsx
  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/services/preferences/uiPreferences.server.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
  • apps/webapp/app/components/TimezoneSetter.tsx
  • apps/webapp/app/root.tsx
  • apps/webapp/app/components/code/CodeBlock.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
  • apps/webapp/app/components/code/TSQLEditor.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

  • apps/webapp/app/routes/resources.timezone.ts
  • apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/components/logs/LogDetailView.tsx
  • apps/webapp/app/components/code/codeMirrorSetup.ts
  • apps/webapp/app/components/primitives/DateTime.tsx
  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/services/preferences/uiPreferences.server.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
  • apps/webapp/app/components/TimezoneSetter.tsx
  • apps/webapp/app/root.tsx
  • apps/webapp/app/components/code/CodeBlock.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
  • apps/webapp/app/components/code/TSQLEditor.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

Import from @trigger.dev/core using subpaths only, never import from root

Files:

  • apps/webapp/app/routes/resources.timezone.ts
  • apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/components/logs/LogDetailView.tsx
  • apps/webapp/app/components/code/codeMirrorSetup.ts
  • apps/webapp/app/components/primitives/DateTime.tsx
  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/services/preferences/uiPreferences.server.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
  • apps/webapp/app/components/TimezoneSetter.tsx
  • apps/webapp/app/root.tsx
  • apps/webapp/app/components/code/CodeBlock.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
  • apps/webapp/app/components/code/TSQLEditor.tsx
apps/webapp/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access all environment variables through the env export of env.server.ts instead of directly accessing process.env in the Trigger.dev webapp

Files:

  • apps/webapp/app/routes/resources.timezone.ts
  • apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/components/logs/LogDetailView.tsx
  • apps/webapp/app/components/code/codeMirrorSetup.ts
  • apps/webapp/app/components/primitives/DateTime.tsx
  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/services/preferences/uiPreferences.server.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
  • apps/webapp/app/components/TimezoneSetter.tsx
  • apps/webapp/app/root.tsx
  • apps/webapp/app/components/code/CodeBlock.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
  • apps/webapp/app/components/code/TSQLEditor.tsx
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: When importing from @trigger.dev/core in the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Access environment variables via env export from apps/webapp/app/env.server.ts, never use process.env directly

Files:

  • apps/webapp/app/routes/resources.timezone.ts
  • apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/components/logs/LogDetailView.tsx
  • apps/webapp/app/components/code/codeMirrorSetup.ts
  • apps/webapp/app/components/primitives/DateTime.tsx
  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/services/preferences/uiPreferences.server.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
  • apps/webapp/app/components/TimezoneSetter.tsx
  • apps/webapp/app/root.tsx
  • apps/webapp/app/components/code/CodeBlock.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
  • apps/webapp/app/components/code/TSQLEditor.tsx
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)

**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries

Files:

  • apps/webapp/app/routes/resources.timezone.ts
  • apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/components/code/codeMirrorSetup.ts
  • apps/webapp/app/services/preferences/uiPreferences.server.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier before committing

Files:

  • apps/webapp/app/routes/resources.timezone.ts
  • apps/webapp/app/presenters/v3/LogsListPresenter.server.ts
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/components/logs/LogDetailView.tsx
  • apps/webapp/app/components/code/codeMirrorSetup.ts
  • apps/webapp/app/components/primitives/DateTime.tsx
  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/services/preferences/uiPreferences.server.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
  • apps/webapp/app/components/TimezoneSetter.tsx
  • apps/webapp/app/root.tsx
  • apps/webapp/app/components/code/CodeBlock.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
  • apps/webapp/app/components/code/TSQLEditor.tsx
apps/webapp/app/services/**/*.server.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Separate testable services from configuration files; follow the pattern of realtimeClient.server.ts (testable service) and realtimeClientGlobal.server.ts (configuration) in the webapp

Files:

  • apps/webapp/app/services/preferences/uiPreferences.server.ts
🧠 Learnings (10)
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Applied to files:

  • apps/webapp/app/routes/resources.timezone.ts
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/components/TimezoneSetter.tsx
  • apps/webapp/app/root.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Scope idempotency keys globally or to current run using the scope parameter

Applied to files:

  • apps/webapp/app/hooks/useShortcutKeys.tsx
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (like the Edit button with PencilSquareIcon) intentionally have no text labels - only icons are shown in the TableCellMenu. This is a deliberate UI design pattern for compact icon-only menu items.

Applied to files:

  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/services/**/*.server.{ts,tsx} : Separate testable services from configuration files; follow the pattern of `realtimeClient.server.ts` (testable service) and `realtimeClientGlobal.server.ts` (configuration) in the webapp

Applied to files:

  • apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from `trigger.dev/core` in the webapp, use subpath exports from the package.json instead of importing from the root path

Applied to files:

  • apps/webapp/app/root.tsx
📚 Learning: 2026-01-28T14:15:15.011Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2953
File: apps/webapp/app/components/runs/v3/SharedFilters.tsx:441-452
Timestamp: 2026-01-28T14:15:15.011Z
Learning: In apps/webapp/app/components/runs/v3/SharedFilters.tsx, the maxPeriodDays limit for date ranges should only check the from date (via dateRangeToDays(fromValue)) because it enforces data retention limits—how far back in history queries can reach. The to date is irrelevant for retention-based limits.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: The webapp at apps/webapp is a Remix 2.1 application using Node.js v20

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
🧬 Code graph analysis (5)
apps/webapp/app/routes/resources.timezone.ts (2)
packages/core/src/v3/apps/http.ts (1)
  • json (65-75)
apps/webapp/app/services/preferences/uiPreferences.server.ts (2)
  • setTimezonePreference (52-56)
  • uiPreferencesStorage (4-14)
apps/webapp/app/components/logs/LogsTable.tsx (2)
apps/webapp/app/components/primitives/DateTime.tsx (1)
  • DateTimeAccurate (321-321)
apps/webapp/app/components/primitives/Buttons.tsx (1)
  • Button (296-329)
apps/webapp/app/components/logs/LogDetailView.tsx (1)
apps/webapp/app/components/primitives/DateTime.tsx (1)
  • DateTimeAccurate (321-321)
apps/webapp/app/services/preferences/uiPreferences.server.ts (1)
internal-packages/tsql/src/query/printer_context.ts (1)
  • timezone (124-126)
apps/webapp/app/components/TimezoneSetter.tsx (1)
apps/webapp/app/root.tsx (1)
  • loader (50-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: sdk-compat / Bun Runtime
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
  • GitHub Check: sdk-compat / Cloudflare Workers
🔇 Additional comments (23)
apps/webapp/app/hooks/useShortcutKeys.tsx (2)

26-34: LGTM!

The scopes parameter addition is well-typed and properly defaulted to "global" for backward compatibility. This enables callers to optionally scope shortcuts to specific UI contexts when needed.


46-61: LGTM!

Good defensive change. The event.repeat guard correctly prevents the action from firing multiple times when a key is held down, which improves UX for shortcuts that should only trigger once per keypress. The scopes option is properly forwarded to the underlying useHotkeys call.

apps/webapp/app/env.server.ts (1)

1179-1179: LGTM!

The increase from 256MB to 1GB for the default ClickHouse logs list memory usage is appropriate to support more complex log queries. The value remains configurable via environment variable for operators with different resource constraints.

apps/webapp/app/components/code/codeMirrorSetup.ts (1)

2-43: Clean history + undo/redo integration.

The history extension and high-precedence undo/redo bindings look solid and should keep editor behavior consistent.

apps/webapp/app/components/code/TSQLEditor.tsx (4)

4-4: Import change aligns with new keymap usage.


63-87: Toggle handler is clear and preserves indentation.


162-168: Shortcut bindings are well-scoped and readable.


255-257: Focusing the editor on container click improves UX.

apps/webapp/app/components/code/CodeBlock.tsx (1)

503-520: Good: ref is applied in both loading and highlighted render paths.

This keeps the container reference consistent across the Prism load transition.

apps/webapp/app/components/TimezoneSetter.tsx (1)

1-30: LGTM — one-time timezone sync is clear.

No issues spotted.

apps/webapp/app/services/preferences/uiPreferences.server.ts (1)

46-55: LGTM — preference accessors are straightforward.

Looks good.

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts (1)

11-11: No issues with the import addition.

All good.

apps/webapp/app/root.tsx (2)

13-72: LGTM — loader now includes timezone in root data.

Nice wiring of timezone preference into the loader payload.


123-126: LGTM — TimezoneSetter placement looks good.

No concerns.

apps/webapp/app/presenters/v3/LogsListPresenter.server.ts (1)

352-359: LGTM — tighter LOG_INFO exclusion looks good.

This keeps internal info logs out of the default view without affecting other kinds.

apps/webapp/app/components/primitives/DateTime.tsx (3)

181-198: Nice — SmartDateTime now respects the user’s timezone.


249-277: LGTM — displayTimeZone keeps DateTimeAccurate consistent.


344-349: LGTM — DateTimeShort is aligned with the user’s timezone.

apps/webapp/app/components/logs/LogDetailView.tsx (1)

11-12: LGTM — DateTimeAccurate ensures precise, timezone-aware timestamps in details.

Also applies to: 234-238

apps/webapp/app/components/logs/LogsTable.tsx (2)

1-37: LGTM — new onCheckForMore prop is cleanly plumbed.

Also applies to: 59-69


157-166: Good swap to DateTimeAccurate for row timestamps.

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (2)

16-18: LGTM — LogEntry import consolidation is clean.


155-170: LGTM — retention limit is consistently wired into loader + filters.

Also applies to: 174-242, 264-317

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@mpcgrid mpcgrid marked this pull request as ready for review February 4, 2026 15:22
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 6 additional flags.

Open in Devin Review

@mpcgrid mpcgrid changed the title Multiple Multiple UI/UX fixes -feb Feb 4, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)

1644-1654: ⚠️ Potential issue | 🟡 Minor

AdjacentRunsShortcuts still displays [ / ] but the actual bindings are now j / k.

The shortcut hints in the bottom toolbar are stale. They should be updated to match the new keybindings.

Proposed fix
 function AdjacentRunsShortcuts() {
   return (
     <div className="flex items-center gap-0.5">
-      <ShortcutKey shortcut={{ key: "[" }} variant="medium" className="ml-0 mr-0 px-1" />
-      <ShortcutKey shortcut={{ key: "]" }} variant="medium" className="ml-0 mr-0 px-1" />
+      <ShortcutKey shortcut={{ key: "j" }} variant="medium" className="ml-0 mr-0 px-1" />
+      <ShortcutKey shortcut={{ key: "k" }} variant="medium" className="ml-0 mr-0 px-1" />
       <Paragraph variant="extra-small" className="ml-1.5 whitespace-nowrap">
         Next/previous run
       </Paragraph>
     </div>
   );
 }
🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/code/TSQLEditor.tsx`:
- Around line 64-74: The selection end line can be wrong when the
caret/selection `to` is exactly at the start of a line: `doc.lineAt(to)` will
return that next line and cause an extra line to be toggled. In
`toggleLineComment` adjust the `to` used to compute `endLine` by detecting the
edge case (if `to > from` and `view.state.doc.lineAt(to).from === to`) and use
`to - 1` (or the previous character position) instead when calling `doc.lineAt`;
update `endLine`/the loop to use that adjusted position so the line collection
excludes the unintended extra line. Reference: toggleLineComment,
view.state.selection.main, startLine, endLine, doc.lineAt, lines.
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5f2ad3 and e622d1b.

📒 Files selected for processing (3)
  • apps/webapp/app/components/Shortcuts.tsx
  • apps/webapp/app/components/code/TSQLEditor.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

**/*.{ts,tsx}: Always import tasks from @trigger.dev/sdk, never use @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Every Trigger.dev task must be exported and have a unique id property with no timeouts in the run function

Files:

  • apps/webapp/app/components/code/TSQLEditor.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
  • apps/webapp/app/components/Shortcuts.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

  • apps/webapp/app/components/code/TSQLEditor.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
  • apps/webapp/app/components/Shortcuts.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

Import from @trigger.dev/core using subpaths only, never import from root

Files:

  • apps/webapp/app/components/code/TSQLEditor.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
  • apps/webapp/app/components/Shortcuts.tsx
apps/webapp/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access all environment variables through the env export of env.server.ts instead of directly accessing process.env in the Trigger.dev webapp

Files:

  • apps/webapp/app/components/code/TSQLEditor.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
  • apps/webapp/app/components/Shortcuts.tsx
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: When importing from @trigger.dev/core in the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Access environment variables via env export from apps/webapp/app/env.server.ts, never use process.env directly

Files:

  • apps/webapp/app/components/code/TSQLEditor.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
  • apps/webapp/app/components/Shortcuts.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier before committing

Files:

  • apps/webapp/app/components/code/TSQLEditor.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
  • apps/webapp/app/components/Shortcuts.tsx
🧠 Learnings (4)
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Scope idempotency keys globally or to current run using the scope parameter

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/Shortcuts.tsx (1)
apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
  • ShortcutKey (38-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: sdk-compat / Bun Runtime
  • GitHub Check: sdk-compat / Deno Runtime
  • GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
  • GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: sdk-compat / Cloudflare Workers
🔇 Additional comments (4)
apps/webapp/app/components/Shortcuts.tsx (1)

141-144: LGTM!

Shortcut keys for "Jump to next/previous run" updated to j/k, consistent with the actual keybindings in the route file's PreviousRunButton and NextRunButton.

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)

1825-1825: LGTM!

Shortcut keys updated from [/] to j/k for previous/next run navigation, consistent with the shortcuts help panel.

Also applies to: 1844-1844

apps/webapp/app/components/code/TSQLEditor.tsx (2)

181-187: LGTM!

Comment-toggle keybindings correctly cover both macOS (Cmd-/) and Windows/Linux (Ctrl-/), and are appropriately placed outside the schema-conditional block so they work regardless of schema presence.


274-276: LGTM!

Focusing the editor view on container click ensures clicks on blank areas (padding, overflow) bring focus to the editor. CodeMirror's internal click handling takes care of cursor positioning within the editable region.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 10 additional findings in Devin Review.

Open in Devin Review

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx:
- Around line 397-423: When prepending results in the "check for new" branch
(isCheckingForNewRef in the useEffect), detect if the fetched first-page has a
gap vs the current accumulated logs by comparing timestamps: take the oldest
returned log (last element of fetcher.data.logs) and the newest accumulated log
(prev[0]) and if oldestReturned.timestamp is older than
newestAccumulated.timestamp (i.e. there is a gap), set a visible
flag/notification (e.g., setNeedsReload(true) or call notify) instead of
silently dropping intervening logs; otherwise proceed to dedupe and prepend as
implemented in setAccumulatedLogs. This change should be made inside the
isCheckingForNewRef.current branch where setAccumulatedLogs is called (and
ensure you do not update the pagination cursor in this branch).
🧹 Nitpick comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (2)

293-330: FiltersBar: the list and no-list branches are nearly identical.

Both branches render the same components with the same props (except possibleTasks). Consider extracting the shared filter bar content and only varying the possibleTasks prop to eliminate this duplication.

♻️ Suggested refactor
   return (
     <div className="flex items-start justify-between gap-x-2 border-b border-grid-bright p-2">
       <div className="flex flex-row flex-wrap items-center gap-1">
-        {list ? (
-          <>
-            <LogsTaskFilter possibleTasks={list.possibleTasks} />
-            <LogsRunIdFilter />
-            <TimeFilter defaultPeriod={defaultPeriod} maxPeriodDays={retentionLimitDays} />
-            <LogsLevelFilter />
-            <LogsSearchInput />
-            {hasFilters && (
-              <Form className="h-6">
-                <Button
-                  variant="secondary/small"
-                  LeadingIcon={XMarkIcon}
-                  tooltip="Clear all filters"
-                />
-              </Form>
-            )}
-          </>
-        ) : (
-          <>
-            <LogsTaskFilter possibleTasks={[]} />
-            <LogsRunIdFilter />
-            <TimeFilter defaultPeriod={defaultPeriod} maxPeriodDays={retentionLimitDays} />
-            <LogsLevelFilter />
-            <LogsSearchInput />
-            {hasFilters && (
-              <Form className="h-6">
-                <Button
-                  variant="secondary/small"
-                  LeadingIcon={XMarkIcon}
-                  tooltip="Clear all filters"
-                />
-              </Form>
-            )}
-          </>
-        )}
+        <LogsTaskFilter possibleTasks={list?.possibleTasks ?? []} />
+        <LogsRunIdFilter />
+        <TimeFilter defaultPeriod={defaultPeriod} maxPeriodDays={retentionLimitDays} />
+        <LogsLevelFilter />
+        <LogsSearchInput />
+        {hasFilters && (
+          <Form className="h-6">
+            <Button
+              variant="secondary/small"
+              LeadingIcon={XMarkIcon}
+              tooltip="Clear all filters"
+            />
+          </Form>
+        )}
       </div>

346-353: LogsList destructures only list but declares isAdmin, showDebug, and defaultPeriod in its prop type.

These extra props are passed at the call site (line 245-248) but never consumed inside the component. If they're not needed, remove them from both the type and the call site to avoid confusion.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e622d1b and 7b26f8d.

📒 Files selected for processing (4)
  • apps/webapp/app/components/code/TSQLEditor.tsx
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/webapp/app/components/logs/LogsTable.tsx
  • apps/webapp/app/components/code/TSQLEditor.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

**/*.{ts,tsx}: Always import tasks from @trigger.dev/sdk, never use @trigger.dev/sdk/v3 or deprecated client.defineJob pattern
Every Trigger.dev task must be exported and have a unique id property with no timeouts in the run function

Files:

  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

Import from @trigger.dev/core using subpaths only, never import from root

Files:

  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
apps/webapp/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access all environment variables through the env export of env.server.ts instead of directly accessing process.env in the Trigger.dev webapp

Files:

  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: When importing from @trigger.dev/core in the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Access environment variables via env export from apps/webapp/app/env.server.ts, never use process.env directly

Files:

  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier before committing

Files:

  • apps/webapp/app/hooks/useShortcutKeys.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
🧠 Learnings (6)
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2026-01-28T14:15:15.011Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2953
File: apps/webapp/app/components/runs/v3/SharedFilters.tsx:441-452
Timestamp: 2026-01-28T14:15:15.011Z
Learning: In apps/webapp/app/components/runs/v3/SharedFilters.tsx, the maxPeriodDays limit for date ranges should only check the from date (via dateRangeToDays(fromValue)) because it enforces data retention limits—how far back in history queries can reach. The to date is irrelevant for retention-based limits.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: The webapp at apps/webapp is a Remix 2.1 application using Node.js v20

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.

Applied to files:

  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (26)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: sdk-compat / Cloudflare Workers
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: sdk-compat / Bun Runtime
  • GitHub Check: typecheck / typecheck
  • GitHub Check: sdk-compat / Deno Runtime
🔇 Additional comments (3)
apps/webapp/app/hooks/useShortcutKeys.tsx (1)

46-49: LGTM — clean guard against repeated key events.

Using event.repeat is the correct, standard approach to prevent actions from firing multiple times when a key is held down. Also a nice cleanup dropping the unused hotkeysEvent parameter.

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx (2)

155-169: LGTM — retentionLimitDays wiring through loader.

The fallback to 30 days is a reasonable default, and the value is correctly deferred to the client.


475-485: LGTM — handleCheckForMore implementation.

Correctly sets the isCheckingForNewRef flag, clears cursor/log params, and guards against concurrent fetches.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +397 to +423
// Append/prepend new logs when fetcher completes (with deduplication)
useEffect(() => {
if (fetcher.data && fetcher.state === "idle") {
// Ignore fetcher data if it was loaded for a different filter state
if (fetcherFilterStateRef.current !== location.search) {
return;
}

const existingIds = new Set(accumulatedLogs.map((log) => log.id));
const newLogs = fetcher.data.logs.filter((log) => !existingIds.has(log.id));
if (newLogs.length > 0) {
setAccumulatedLogs((prev) => [...prev, ...newLogs]);
if (isCheckingForNewRef.current) {
// "Check for new" - prepend new logs, don't update cursor
setAccumulatedLogs((prev) => {
const existingIds = new Set(prev.map((log) => log.id));
const newLogs = fetcher.data!.logs.filter((log) => !existingIds.has(log.id));
return newLogs.length > 0 ? [...newLogs, ...prev] : prev;
});
isCheckingForNewRef.current = false;
} else {
// "Load more" - append logs and update cursor
setAccumulatedLogs((prev) => {
const existingIds = new Set(prev.map((log) => log.id));
const newLogs = fetcher.data!.logs.filter((log) => !existingIds.has(log.id));
return newLogs.length > 0 ? [...prev, ...newLogs] : prev;
});
setNextCursor(fetcher.data.pagination.next);
}
setNextCursor(fetcher.data.pagination.next);
}
}, [fetcher.data, fetcher.state, accumulatedLogs, location.search]);
}, [fetcher.data, fetcher.state, location.search]);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Check-for-new may silently drop logs if the gap exceeds one page.

handleCheckForMore fetches the first page (no cursor). If more new logs arrived than fit in a single page, logs between the end of that first page and the start of accumulatedLogs will be missing — and the user won't know. Consider either:

  • Comparing the oldest returned log's timestamp against the newest accumulated log to detect a gap, and signaling the user to reload if one exists.
  • Or documenting this as a known limitation.
🤖 Prompt for AI Agents
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx
around lines 397 - 423, When prepending results in the "check for new" branch
(isCheckingForNewRef in the useEffect), detect if the fetched first-page has a
gap vs the current accumulated logs by comparing timestamps: take the oldest
returned log (last element of fetcher.data.logs) and the newest accumulated log
(prev[0]) and if oldestReturned.timestamp is older than
newestAccumulated.timestamp (i.e. there is a gap), set a visible
flag/notification (e.g., setNeedsReload(true) or call notify) instead of
silently dropping intervening logs; otherwise proceed to dedupe and prepend as
implemented in setAccumulatedLogs. This change should be made inside the
isCheckingForNewRef.current branch where setAccumulatedLogs is called (and
ensure you do not update the pagination cursor in this branch).

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 13 additional findings in Devin Review.

Open in Devin Review

Comment on lines 213 to 218
{!hasMore && logs.length > 0 && (
<div className="flex items-center justify-center py-12">
<div className="flex items-center gap-2">
<div className="flex flex-col items-center gap-3">
<span className="text-text-dimmed">Showing all {logs.length} logs</span>
</div>
</div>

Choose a reason for hiding this comment

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

🔴 onCheckForMore callback is accepted by LogsTable but never invoked

The PR description states "Added a check for new logs button, previously once the user got to the end of the logs he could not check for newer logs." The route component creates handleCheckForMore at route.tsx:475-485 and passes it as onCheckForMore={handleCheckForMore} to LogsTable at route.tsx:498. The LogsTable component accepts and destructures this prop at LogsTable.tsx:66, but never calls it anywhere in the component. There is no button or UI element that triggers onCheckForMore.

Root Cause and Impact

The entire "check for new logs" pipeline is wired up on the backend side: handleCheckForMore builds a URL without a cursor, sets isCheckingForNewRef.current = true, and fires fetcher.load(). The effect at route.tsx:397-423 handles the response by prepending new logs when isCheckingForNewRef.current is true. But the LogsTable component at LogsTable.tsx:59-221 never renders a button or calls onCheckForMore(), so this entire feature is dead code.

The "Show all logs" message area at LogsTable.tsx:213-219 was updated (the diff shows the div changing from flex items-center gap-2 to flex flex-col items-center gap-3), which looks like it was prepared to include a "Check for new" button but the button itself was never added.

Impact: Users who reach the end of their logs list have no way to check for newer logs without manually refreshing the page, which is exactly the problem this feature was supposed to fix.

(Refers to lines 213-219)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

1 participant