Skip to content

[slop]feat(frontend): enable compute, add historical logs#5059

Merged
jog1t merged 1 commit into
mainfrom
05-15-_slop_feat_frontend_enable_compute_add_historical_logs
May 15, 2026
Merged

[slop]feat(frontend): enable compute, add historical logs#5059
jog1t merged 1 commit into
mainfrom
05-15-_slop_feat_frontend_enable_compute_add_historical_logs

Conversation

@abcxff
Copy link
Copy Markdown
Contributor

@abcxff abcxff commented May 15, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@abcxff abcxff mentioned this pull request May 15, 2026
11 tasks
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

PR Review: feat(frontend): enable compute, add historical logs

Overview

This PR re-enables previously-commented-out Rivet Compute features (managed pool tabs, agent instructions button, provider dropdown item, deployments sidebar link) and adds real historical log fetching with a "Back to newest" jump button and frozen-scroll behavior.


Issues

High Priority

Ref mutation during render (concurrent mode hazard)deployment-logs.tsx

The frozenLogsRef and frozenFirstIdRef are mutated directly inside the render body:

if (follow) {
  frozenLogsRef.current = logs;
  frozenFirstIdRef.current = logs[0]?.data.insertId;
} else if (logs[0]?.data.insertId !== frozenFirstIdRef.current) {
  frozenLogsRef.current = logs;
  ...
}

In React concurrent mode, renders can be interrupted and retried with different state. Mutating refs mid-render can leave them in a torn state between attempts. Move this logic into a useEffect or derive it via useMemo + state, using useEffect to commit the freeze when follow transitions from true to false.

Timestamp-based pagination cursoruse-deployment-logs-stream.ts

const before = currentLogs.length > 0
  ? currentLogs[0].data.timestamp
  : new Date().toISOString();

Multiple entries can share the same timestamp, causing entries to be skipped or duplicated. Use the unique insertId of the first visible log as the cursor instead.

Missing abort signal in loadMoreHistory

fetchLogsHistory inside loadMoreHistory has no AbortSignal. If the component unmounts mid-fetch, the response will arrive and call state setters on a ghost. Pass a component-level AbortController signal (or create a local one per call and cancel it in cleanup).

Medium Priority

Silent error swallowing on initial history fetch

} catch {
  // Non-fatal. The stream will still start.
}

This silently drops fetch errors. At minimum, log the error to the console for debuggability. Ideally surface it transiently so developers know the seed fetch failed.

loadMoreHistory errors not surfaced to users

} catch (err) {
  console.error("Failed to load historical logs:", err);
}

Only console-logged. Call setError (or a local toast) so users know the "load more" action failed and can retry.

isLoadingMore in useCallback deps causes churn

const loadMoreHistory = useCallback(async () => { ... },
  [isLoadingMore, hasMore, project, namespace, pool, filter, region]);

isLoadingMore flips true then false on every load, creating two new callback references per fetch. Use a useRef for the loading flag inside the guard and remove it from the dep array (the ref read is always fresh). hasMore has the same issue.

Rules of Hooks violation commentactors-actor-details.tsx

// biome-ignore lint/correctness/useHookAtTopLevel: guarded by build constant
const provider = useCloudNamespaceDataProvider();
// biome-ignore lint/correctness/useHookAtTopLevel: guarded by build constant
const { data: hasManagedPool } = useSuspenseQuery(...);

This only works safely if features.platform is a true compile-time dead-code-eliminated constant (never changes between renders). If there is any risk of it being reconfigured at runtime, this will corrupt React's hook call order. Add a comment making the compile-time-constant guarantee explicit, or refactor into two separate components to avoid the lint suppression entirely.

Low Priority

PR-number-based dependency reference is less stable than a commit hash

"@rivet-gg/cloud": "https://pkg.pr.new/rivet-dev/engine-ee/@rivet-gg/cloud@299"

PR @299 can be force-pushed or closed, making the resolved package change without a lockfile bump. Prefer pinning to a commit SHA once the upstream PR is merged.

Lock file shows a Tailwind version mismatch

The snapshot changed from tailwindcss-animate: 1.0.7(tailwindcss@4.2.2) to tailwindcss-animate: 1.0.7(tailwindcss@3.4.18(...)). Verify tailwindcss-animate is tested against the correct major version to avoid subtle style regressions.

logsRef.current = logs during render

const logsRef = useRef(logs);
logsRef.current = logs;

Mutating a ref during render is a code-smell. Since loadMoreHistory closes over the ref, moving this assignment into a useEffect would be cleaner.


Positive Notes

  • The frozen-log UX pattern (stop scroll churn when the user scrolls up) is a good approach for a log viewer.
  • The "Back to newest" jump button is placed correctly and styled consistently with the rest of the UI.
  • Silently seeding with initial history before the stream starts avoids a blank-screen flash on load.
  • Cleaning up the old FIXME/commented-out code reduces noise significantly.

Posted by claude[bot]

@abcxff abcxff requested a review from jog1t May 15, 2026 18:11
@jog1t jog1t changed the base branch from 05-11-fix_frontend_fix_actor_not_showing_up_in_the_actors_list to graphite-base/5059 May 15, 2026 19:29
@jog1t jog1t mentioned this pull request May 15, 2026
11 tasks
@abcxff abcxff marked this pull request as ready for review May 15, 2026 19:33
Copy link
Copy Markdown
Contributor

jog1t commented May 15, 2026

Merge activity

  • May 15, 7:36 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 15, 8:07 PM UTC: Graphite rebased this pull request as part of a merge.
  • May 15, 8:08 PM UTC: @jog1t merged this pull request with Graphite.

@jog1t jog1t changed the base branch from graphite-base/5059 to main May 15, 2026 20:05
@jog1t jog1t force-pushed the 05-15-_slop_feat_frontend_enable_compute_add_historical_logs branch from 5491fed to ab5d87d Compare May 15, 2026 20:06
@jog1t jog1t merged commit 950f9c2 into main May 15, 2026
9 of 12 checks passed
@jog1t jog1t deleted the 05-15-_slop_feat_frontend_enable_compute_add_historical_logs branch May 15, 2026 20:08
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