Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a live "tail" mode to config changes: query hooks accept an optional Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as ConfigChangesPage / ConfigDetailsChangesPage
participant Hook as useGetAllConfigsChangesQuery / useGetConfigChangesByIDQuery
participant API as Server/API
participant State as Component State
User->>UI: Toggle "Live" on
UI->>State: set liveTail = true, init tailCursor from base data
loop every 5s while liveTail
UI->>Hook: fetch with cursor=tailCursor
Hook->>API: request changes since cursor
API-->>Hook: return changes batch
Hook-->>UI: deliver polled changes
UI->>State: append and dedupe tailedChanges by id
UI->>State: advance tailCursor to newest created_at
UI->>UI: merge tailed-only items with base changes and update totals
end
User->>UI: Toggle "Live" off
UI->>State: set liveTail = false, clear tailedChanges, reset tailCursor
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/pages/config/details/ConfigDetailsChangesPage.tsx (1)
11-19:getNewestInsertedAthelper is duplicated across pages.This helper function is identical in both
ConfigChangesPage.tsxandConfigDetailsChangesPage.tsx. Consider extracting it to a shared utility module to improve maintainability.Also applies to: 21-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 11 - 19, The getNewestInsertedAt helper is duplicated; extract it into a shared utility (e.g., export function getNewestInsertedAt(changes: ConfigChange[]): string | undefined) and replace the copies in ConfigDetailsChangesPage and ConfigChangesPage with an import from that new module; ensure the new util exports the same signature and any required types (ConfigChange) or import/export the type from its original location so both components compile without changing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/config/ConfigChangesPage.tsx`:
- Around line 96-118: The mapping uses non-null assertions on optional fields
(config_id, type, name) in baseChanges and tailedWithConfig; replace those
assertions with defensive handling by either filtering out entries missing
required fields before mapping or constructing the config object only when
change.config_id, change.type, and change.name are present (e.g., return
undefined or skip the item if any are missing), and ensure baseIds/uniqueTailed
logic works with the possibly filtered arrays; update the transformation that
builds config (in baseChanges and tailedWithConfig) to handle undefined values
safely rather than using the ! operator.
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx`:
- Around line 80-96: The mapping for baseChanges and tailedWithConfig is using
non-null assertions on change.config_id, change.type, and change.name which can
throw if the API returns incomplete records; update the mapping logic in
ConfigDetailsChangesPage (functions/vars: baseChanges and tailedWithConfig) to
either filter out changes missing required fields (e.g., drop entries where any
of config_id/type/name is null/undefined) or provide safe fallbacks (e.g., id:
change.config_id ?? 'unknown', type: change.type ?? 'unknown', name: change.name
?? 'Unnamed') before constructing the config object so runtime errors are
avoided.
---
Nitpick comments:
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx`:
- Around line 11-19: The getNewestInsertedAt helper is duplicated; extract it
into a shared utility (e.g., export function getNewestInsertedAt(changes:
ConfigChange[]): string | undefined) and replace the copies in
ConfigDetailsChangesPage and ConfigChangesPage with an import from that new
module; ensure the new util exports the same signature and any required types
(ConfigChange) or import/export the type from its original location so both
components compile without changing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d18ca48-c849-4e94-b69c-3b4868d0fe21
📒 Files selected for processing (7)
src/api/query-hooks/useConfigChangesHooks.tssrc/api/services/configs.tssrc/api/types/configs.tssrc/components/Configs/Changes/ConfigChangesFilters/ConfigChangesFilters.tsxsrc/components/Configs/Changes/ConfigsRelatedChanges/FilterBar/ConfigRelatedChangesFilters.tsxsrc/pages/config/ConfigChangesPage.tsxsrc/pages/config/details/ConfigDetailsChangesPage.tsx
Comment on lines
+96
to
+118
| const baseChanges = (data?.changes ?? []).map((change) => ({ | ||
| ...change, | ||
| config: { | ||
| id: change.config_id!, | ||
| type: change.type!, | ||
| name: change.name!, | ||
| deleted_at: change.deleted_at | ||
| } | ||
| })); | ||
|
|
||
| const tailedWithConfig = tailedChanges.map((change) => ({ | ||
| ...change, | ||
| config: { | ||
| id: changes.config_id!, | ||
| type: changes.type!, | ||
| name: changes.name!, | ||
| deleted_at: changes.deleted_at | ||
| id: change.config_id!, | ||
| type: change.type!, | ||
| name: change.name!, | ||
| deleted_at: change.deleted_at | ||
| } | ||
| })); | ||
|
|
||
| const totalChanges = data?.total ?? 0; | ||
| const baseIds = new Set(baseChanges.map((c) => c.id)); | ||
| const uniqueTailed = tailedWithConfig.filter((c) => !baseIds.has(c.id)); | ||
| const changes = [...uniqueTailed, ...baseChanges]; |
Contributor
There was a problem hiding this comment.
Same non-null assertion concern as ConfigDetailsChangesPage.tsx.
The mapping logic uses non-null assertions on optional fields (config_id, type, name). Apply the same defensive handling as suggested for the other page.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/ConfigChangesPage.tsx` around lines 96 - 118, The mapping
uses non-null assertions on optional fields (config_id, type, name) in
baseChanges and tailedWithConfig; replace those assertions with defensive
handling by either filtering out entries missing required fields before mapping
or constructing the config object only when change.config_id, change.type, and
change.name are present (e.g., return undefined or skip the item if any are
missing), and ensure baseIds/uniqueTailed logic works with the possibly filtered
arrays; update the transformation that builds config (in baseChanges and
tailedWithConfig) to handle undefined values safely rather than using the !
operator.
Comment on lines
+80
to
+96
| const baseChanges = (data?.changes ?? []).map((change) => ({ | ||
| ...change, | ||
| config: { | ||
| id: changes.config_id!, | ||
| type: changes.type!, | ||
| name: changes.name! | ||
| id: change.config_id!, | ||
| type: change.type!, | ||
| name: change.name! | ||
| } | ||
| })); | ||
|
|
||
| const totalChanges = data?.total ?? 0; | ||
| const tailedWithConfig = tailedChanges.map((change) => ({ | ||
| ...change, | ||
| config: { | ||
| id: change.config_id!, | ||
| type: change.type!, | ||
| name: change.name! | ||
| } | ||
| })); |
Contributor
There was a problem hiding this comment.
Non-null assertions may cause runtime errors if API returns incomplete data.
The code uses non-null assertions (!) on config_id, type, and name properties which are typed as optional in ConfigChange. If the API returns changes without these fields, this will result in undefined values being assigned.
Consider adding fallback values or filtering out incomplete records:
Proposed defensive handling
const baseChanges = (data?.changes ?? []).map((change) => ({
...change,
config: {
- id: change.config_id!,
- type: change.type!,
- name: change.name!
+ id: change.config_id ?? "",
+ type: change.type ?? "",
+ name: change.name ?? ""
}
}));
const tailedWithConfig = tailedChanges.map((change) => ({
...change,
config: {
- id: change.config_id!,
- type: change.type!,
- name: change.name!
+ id: change.config_id ?? "",
+ type: change.type ?? "",
+ name: change.name ?? ""
}
}));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const baseChanges = (data?.changes ?? []).map((change) => ({ | |
| ...change, | |
| config: { | |
| id: changes.config_id!, | |
| type: changes.type!, | |
| name: changes.name! | |
| id: change.config_id!, | |
| type: change.type!, | |
| name: change.name! | |
| } | |
| })); | |
| const totalChanges = data?.total ?? 0; | |
| const tailedWithConfig = tailedChanges.map((change) => ({ | |
| ...change, | |
| config: { | |
| id: change.config_id!, | |
| type: change.type!, | |
| name: change.name! | |
| } | |
| })); | |
| const baseChanges = (data?.changes ?? []).map((change) => ({ | |
| ...change, | |
| config: { | |
| id: change.config_id ?? "", | |
| type: change.type ?? "", | |
| name: change.name ?? "" | |
| } | |
| })); | |
| const tailedWithConfig = tailedChanges.map((change) => ({ | |
| ...change, | |
| config: { | |
| id: change.config_id ?? "", | |
| type: change.type ?? "", | |
| name: change.name ?? "" | |
| } | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 80 - 96,
The mapping for baseChanges and tailedWithConfig is using non-null assertions on
change.config_id, change.type, and change.name which can throw if the API
returns incomplete records; update the mapping logic in ConfigDetailsChangesPage
(functions/vars: baseChanges and tailedWithConfig) to either filter out changes
missing required fields (e.g., drop entries where any of config_id/type/name is
null/undefined) or provide safe fallbacks (e.g., id: change.config_id ??
'unknown', type: change.type ?? 'unknown', name: change.name ?? 'Unnamed')
before constructing the config object so runtime errors are avoided.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx`:
- Around line 55-70: The live-polling query useGetConfigChangesByIDQuery
currently respects table pagination so setTailCursor can skip unseen rows;
modify the tail fetch to ignore normal pagination or fully drain all delta pages
before advancing the cursor: either (A) update the call-site to pass explicit
pagination override flags (e.g., pageIndex: 0 and pageSize: a large value or a
new ignorePagination: true option) to useGetConfigChangesByIDQuery so the poll
returns the entire delta, or (B) implement a drain loop after receiving pollData
in ConfigDetailsChangesPage that repeatedly requests subsequent pages from
useGetConfigChangesByIDQuery (or the underlying fetch function from
useConfigChangesHooks) until no more changes remain, then compute newest via
getNewestInsertedAt and call setTailCursor with that newest value; ensure this
change references useGetConfigChangesByIDQuery, tailCursor, liveTail,
getNewestInsertedAt, and setTailCursor so the tail cursor only advances after
all delta pages are consumed.
- Around line 47-53: When liveTail is active the tailed state is not reset when
query-shaping parameters change, causing mixing of rows and wrong cursors;
update the useEffect that currently depends only on liveTail to also depend on
the query-shaping params (e.g., filters, sort, time range, config id or the hook
inputs that derive the base query) and call setTailedChanges([]) and
setTailCursor(undefined) whenever any of those params change, and apply the same
change to the other similar effect block referenced in the comment so the tail
state is reseeded whenever the query context updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ad3e5a2c-1372-4a22-8725-11f2c7d9bd96
📒 Files selected for processing (3)
src/api/query-hooks/useConfigChangesHooks.tssrc/pages/config/ConfigChangesPage.tsxsrc/pages/config/details/ConfigDetailsChangesPage.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/config/ConfigChangesPage.tsx
- src/api/query-hooks/useConfigChangesHooks.ts
Comment on lines
+47
to
+53
| // Reset when live tail is turned off | ||
| useEffect(() => { | ||
| if (!liveTail) { | ||
| setTailedChanges([]); | ||
| setTailCursor(undefined); | ||
| } | ||
| }, [liveTail]); |
Contributor
There was a problem hiding this comment.
Reset live-tail state when the query context changes.
While liveTail is on, the base query is disabled but the filter bar stays active. Changing filters/sort/time range/config id updates the hook inputs, yet tailedChanges and tailCursor are only cleared when liveTail flips to false. That can mix rows from the previous query into the new one and poll from the wrong cursor.
Please clear/reseed the tail state whenever the query-shaping params change, or disable those controls while live mode is active.
Also applies to: 123-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 47 - 53,
When liveTail is active the tailed state is not reset when query-shaping
parameters change, causing mixing of rows and wrong cursors; update the
useEffect that currently depends only on liveTail to also depend on the
query-shaping params (e.g., filters, sort, time range, config id or the hook
inputs that derive the base query) and call setTailedChanges([]) and
setTailCursor(undefined) whenever any of those params change, and apply the same
change to the other similar effect block referenced in the comment so the tail
state is reseeded whenever the query context updates.
Comment on lines
+55
to
+70
| const { data: pollData } = useGetConfigChangesByIDQuery({ | ||
| from_inserted_at: tailCursor, | ||
| keepPreviousData: false, | ||
| enabled: liveTail && !!id && !!tailCursor, | ||
| refetchInterval: liveTail ? 5000 : false | ||
| }); | ||
|
|
||
| const changes = (data?.changes ?? []).map((changes) => ({ | ||
| ...changes, | ||
| // Accumulate new items from poll and advance cursor | ||
| useEffect(() => { | ||
| if (!pollData?.changes?.length) return; | ||
|
|
||
| const incoming = pollData.changes; | ||
| const newest = getNewestInsertedAt(incoming); | ||
| if (newest) { | ||
| setTailCursor((prev) => (!prev || newest > prev ? newest : prev)); | ||
| } |
Contributor
There was a problem hiding this comment.
Live polling can permanently skip changes.
useGetConfigChangesByIDQuery still applies the table's pageIndex/pageSize when from_inserted_at is set (src/api/query-hooks/useConfigChangesHooks.ts:152-165). In live mode that means this fetch only returns one page of the delta. If the user is not on page 0, or more than one page of changes arrives between polls, Line 69 advances tailCursor past unseen rows and they are never fetched.
Please make the tail query ignore normal pagination, or drain all delta pages before moving the cursor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 55 - 70,
The live-polling query useGetConfigChangesByIDQuery currently respects table
pagination so setTailCursor can skip unseen rows; modify the tail fetch to
ignore normal pagination or fully drain all delta pages before advancing the
cursor: either (A) update the call-site to pass explicit pagination override
flags (e.g., pageIndex: 0 and pageSize: a large value or a new ignorePagination:
true option) to useGetConfigChangesByIDQuery so the poll returns the entire
delta, or (B) implement a drain loop after receiving pollData in
ConfigDetailsChangesPage that repeatedly requests subsequent pages from
useGetConfigChangesByIDQuery (or the underlying fetch function from
useConfigChangesHooks) until no more changes remain, then compute newest via
getNewestInsertedAt and call setTailCursor with that newest value; ensure this
change references useGetConfigChangesByIDQuery, tailCursor, liveTail,
getNewestInsertedAt, and setTailCursor so the tail cursor only advances after
all delta pages are consumed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/config/ConfigChangesPage.tsx`:
- Around line 56-60: The bug seeds tailCursor from the currently loaded page
(getNewestCreatedAt(data.changes)), causing “new” rows when Live is enabled on
page >1; change the logic in the useEffect that watches liveTail to instead
determine the true head of the filtered result set (not the current page) —
e.g., when liveTail becomes true call the existing data-fetching logic or a
lightweight API endpoint to fetch the newest change for the current filters
(page=1 sorted by created_at desc or a dedicated newestChangedAt query), then
setTailCursor to that returned created_at; update the useEffect around
liveTail/tailCursor/setTailCursor/getNewestCreatedAt to use that server-derived
newest timestamp so enabling Live never replays older pages as new.
- Around line 21-29: The tail cursor is being computed from created_at but must
use inserted_at; update getNewestCreatedAt to read and compare c.inserted_at
(returning inserted_at as string | undefined) instead of c.created_at, and
replace any other occurrences that derive the tail cursor from created_at (the
similar logic at the other two locations referenced) so all cursor computation
and comparisons consistently use inserted_at.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e131e5a8-4b29-47f3-be50-9e711de6cf28
📒 Files selected for processing (3)
src/api/query-hooks/useConfigChangesHooks.tssrc/pages/config/ConfigChangesPage.tsxsrc/pages/config/details/ConfigDetailsChangesPage.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/config/details/ConfigDetailsChangesPage.tsx
- src/api/query-hooks/useConfigChangesHooks.ts
Comment on lines
+21
to
+29
| function getNewestCreatedAt(changes: ConfigChange[]): string | undefined { | ||
| return changes.reduce( | ||
| (latest, c) => | ||
| c.created_at && (!latest || c.created_at > latest) | ||
| ? c.created_at | ||
| : latest, | ||
| undefined as string | undefined | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Use inserted_at for the tail cursor.
The live stream is advancing on created_at, but this feature is supposed to fetch deltas by inserted_at. With the current cursor, a later-inserted row whose created_at is older than the cursor is silently missed.
Suggested fix
-function getNewestCreatedAt(changes: ConfigChange[]): string | undefined {
+function getNewestInsertedAt(changes: ConfigChange[]): string | undefined {
return changes.reduce(
(latest, c) =>
- c.created_at && (!latest || c.created_at > latest)
- ? c.created_at
+ c.inserted_at && (!latest || c.inserted_at > latest)
+ ? c.inserted_at
: latest,
undefined as string | undefined
);
}
@@
- setTailCursor(getNewestCreatedAt(data.changes));
+ setTailCursor(getNewestInsertedAt(data.changes));
@@
- const newest = getNewestCreatedAt(incoming);
+ const newest = getNewestInsertedAt(incoming);Also applies to: 58-60, 83-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/ConfigChangesPage.tsx` around lines 21 - 29, The tail cursor
is being computed from created_at but must use inserted_at; update
getNewestCreatedAt to read and compare c.inserted_at (returning inserted_at as
string | undefined) instead of c.created_at, and replace any other occurrences
that derive the tail cursor from created_at (the similar logic at the other two
locations referenced) so all cursor computation and comparisons consistently use
inserted_at.
Comment on lines
+56
to
+60
| // Initialize cursor from base data when live tail is turned on | ||
| useEffect(() => { | ||
| if (liveTail && data?.changes?.length && !tailCursor) { | ||
| setTailCursor(getNewestCreatedAt(data.changes)); | ||
| } |
Contributor
There was a problem hiding this comment.
Live mode is incorrect on page 2+.
tailCursor is seeded from the newest row in the currently loaded page, not from the actual head of the filtered result set. If someone enables Live while viewing an older page, the first poll will replay rows from earlier pages as "new" and distort both the table and the derived totals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/ConfigChangesPage.tsx` around lines 56 - 60, The bug seeds
tailCursor from the currently loaded page (getNewestCreatedAt(data.changes)),
causing “new” rows when Live is enabled on page >1; change the logic in the
useEffect that watches liveTail to instead determine the true head of the
filtered result set (not the current page) — e.g., when liveTail becomes true
call the existing data-fetching logic or a lightweight API endpoint to fetch the
newest change for the current filters (page=1 sorted by created_at desc or a
dedicated newestChangedAt query), then setTailCursor to that returned
created_at; update the useEffect around
liveTail/tailCursor/setTailCursor/getNewestCreatedAt to use that server-derived
newest timestamp so enabling Live never replays older pages as new.
Comment on lines
+63
to
+69
| // Reset when live tail is turned off | ||
| useEffect(() => { | ||
| if (!liveTail) { | ||
| setTailedChanges([]); | ||
| setTailCursor(undefined); | ||
| } | ||
| }, [liveTail]); |
Contributor
There was a problem hiding this comment.
Reset tail state when the query scope changes.
The tail state is only cleared when liveTail turns off. If filters or pagination change while it stays on, tailedChanges and tailCursor carry over from the previous scope, which can mix results from different queries and skip valid rows in the new scope.
Also applies to: 71-76
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/config/ConfigChangesPage.tsx (1)
71-76:⚠️ Potential issue | 🟠 MajorPoll query still applies table pagination, which can skip changes.
The underlying
useGetAllConfigsChangesQueryhook (lines 69-70 ofuseConfigChangesHooks.ts) passespageIndexandpageSizeto the query even whencursoris set. If the user is viewing page 2+ or if more than one page of delta changes arrives between polls, rows will be permanently skipped.Consider having the poll query ignore pagination by passing explicit overrides (e.g.,
pageIndex: 0, largepageSize), or drain all delta pages before advancing the cursor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/ConfigChangesPage.tsx` around lines 71 - 76, The poll query using useGetAllConfigsChangesQuery with cursor/tail (called with tailCursor and liveTail) still applies pagination (pageIndex/pageSize) from useConfigChangesHooks.ts which can skip rows; update the poll invocation or the hook so that when cursor is provided the query overrides pagination (e.g., pass pageIndex: 0 and a sufficiently large pageSize or a sentinel to fetch all deltas) or change the hook logic in useConfigChangesHooks.ts to ignore pageIndex/pageSize when cursor is set and iterate/drain all delta pages before advancing tailCursor; make the change around the useGetAllConfigsChangesQuery call and the pagination logic that references pageIndex/pageSize so polling returns all changes.
🧹 Nitpick comments (1)
src/pages/config/ConfigChangesPage.tsx (1)
78-93: Consider extracting live tail logic to a shared hook.The live tail implementation (state management, cursor initialization, polling accumulation) is nearly identical between
ConfigChangesPageandConfigDetailsChangesPage. Extracting this to a reusable hook likeuseLiveTailChangeswould reduce duplication and ensure consistent behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/ConfigChangesPage.tsx` around lines 78 - 93, Extract the duplicated live-tail logic into a reusable hook named useLiveTailChanges and replace the in-component logic in ConfigChangesPage and ConfigDetailsChangesPage with calls to that hook; the hook should accept pollData (the structure containing changes), use getNewestCreatedAt internally to advance and set tail cursor, maintain and expose tailCursor and tailedChanges state plus any setters needed, and implement the same accumulation/filtering behavior currently done with setTailCursor and setTailedChanges so both components consume the shared hook instead of duplicating the useEffect logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx`:
- Around line 11-19: The helper getNewestCreatedAt currently computes the tail
cursor using created_at; change it to use the inserted_at field instead (e.g.,
rename to getNewestInsertedAt or update its logic) so the reduce compares
c.inserted_at and returns the newest inserted_at string (handle missing values
the same way as before). Then update all call sites that use getNewestCreatedAt
(the two places where the tail cursor is built) to call the new/updated helper
so the delta fetch uses inserted_at rather than created_at.
---
Duplicate comments:
In `@src/pages/config/ConfigChangesPage.tsx`:
- Around line 71-76: The poll query using useGetAllConfigsChangesQuery with
cursor/tail (called with tailCursor and liveTail) still applies pagination
(pageIndex/pageSize) from useConfigChangesHooks.ts which can skip rows; update
the poll invocation or the hook so that when cursor is provided the query
overrides pagination (e.g., pass pageIndex: 0 and a sufficiently large pageSize
or a sentinel to fetch all deltas) or change the hook logic in
useConfigChangesHooks.ts to ignore pageIndex/pageSize when cursor is set and
iterate/drain all delta pages before advancing tailCursor; make the change
around the useGetAllConfigsChangesQuery call and the pagination logic that
references pageIndex/pageSize so polling returns all changes.
---
Nitpick comments:
In `@src/pages/config/ConfigChangesPage.tsx`:
- Around line 78-93: Extract the duplicated live-tail logic into a reusable hook
named useLiveTailChanges and replace the in-component logic in ConfigChangesPage
and ConfigDetailsChangesPage with calls to that hook; the hook should accept
pollData (the structure containing changes), use getNewestCreatedAt internally
to advance and set tail cursor, maintain and expose tailCursor and tailedChanges
state plus any setters needed, and implement the same accumulation/filtering
behavior currently done with setTailCursor and setTailedChanges so both
components consume the shared hook instead of duplicating the useEffect logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 267236dd-aace-487b-baa5-b69f5413262c
📒 Files selected for processing (2)
src/pages/config/ConfigChangesPage.tsxsrc/pages/config/details/ConfigDetailsChangesPage.tsx
Comment on lines
+11
to
+19
| function getNewestCreatedAt(changes: ConfigChange[]): string | undefined { | ||
| return changes.reduce<string | undefined>( | ||
| (latest, c) => | ||
| c.created_at && (!latest || c.created_at > latest) | ||
| ? c.created_at | ||
| : latest, | ||
| undefined | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Use inserted_at instead of created_at for the tail cursor.
Per the PR objectives, the delta fetch should use inserted_at to identify new entries. Using created_at can miss rows that were inserted after the cursor timestamp but have an older created_at value.
Suggested fix
-function getNewestCreatedAt(changes: ConfigChange[]): string | undefined {
+function getNewestInsertedAt(changes: ConfigChange[]): string | undefined {
return changes.reduce<string | undefined>(
(latest, c) =>
- c.created_at && (!latest || c.created_at > latest)
- ? c.created_at
+ c.inserted_at && (!latest || c.inserted_at > latest)
+ ? c.inserted_at
: latest,
undefined
);
}Update the call sites at lines 43 and 67 accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 11 - 19,
The helper getNewestCreatedAt currently computes the tail cursor using
created_at; change it to use the inserted_at field instead (e.g., rename to
getNewestInsertedAt or update its logic) so the reduce compares c.inserted_at
and returns the newest inserted_at string (handle missing values the same way as
before). Then update all call sites that use getNewestCreatedAt (the two places
where the tail cursor is built) to call the new/updated helper so the delta
fetch uses inserted_at rather than created_at.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
src/pages/config/ConfigChangesPage.tsx (4)
57-60:⚠️ Potential issue | 🟠 MajorBootstrapping from the loaded slice breaks Live mode.
data.changesis only the currently loaded slice, and polling is disabled untiltailCursorexists. So Live can never start from an empty result set, and starting it from an older slice can replay already-existing rows as “new”. Seed from a server-derived head for the active filters, or from a server-side “now” when there are no loaded rows.Also applies to: 72-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/ConfigChangesPage.tsx` around lines 57 - 60, The current useEffect bootstraps tailCursor from the currently loaded slice (data.changes) which prevents Live from starting when the slice is empty or causes replay of older rows; change the initialization in the useEffect that checks liveTail to prefer a server-derived head (for the active filters) and only fall back to getNewestCreatedAt(data.changes) when that head is unavailable, and if there are no loaded rows use a server-side “now” timestamp to seed tailCursor before enabling polling; update both occurrences that set tailCursor (the useEffect around liveTail and the duplicate block at the later lines) to call the server endpoint/prop for the head or now and then call setTailCursor with that value.
21-29:⚠️ Potential issue | 🟠 MajorAdvance the live cursor on
inserted_at, notcreated_at.The polling path still derives
tailCursorfromcreated_at, but the delta contract for this PR is based oninserted_at. Late-inserted rows with oldercreated_atvalues will be skipped.Suggested fix
-function getNewestCreatedAt(changes: ConfigChange[]): string | undefined { +function getNewestInsertedAt(changes: ConfigChange[]): string | undefined { let latest: string | undefined; for (const c of changes) { - const ts = typeof c.created_at === "string" ? c.created_at : undefined; + const ts = typeof c.inserted_at === "string" ? c.inserted_at : undefined; if (ts && (!latest || ts > latest)) { latest = ts; } } return latest; } @@ - setTailCursor(getNewestCreatedAt(data.changes)); + setTailCursor(getNewestInsertedAt(data.changes)); @@ - const newest = getNewestCreatedAt(incoming); + const newest = getNewestInsertedAt(incoming);Also applies to: 59-60, 83-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/ConfigChangesPage.tsx` around lines 21 - 29, The code currently derives the live tail cursor from created_at (function getNewestCreatedAt and any code that sets tailCursor) which skips late-inserted rows; change getNewestCreatedAt to read and compare c.inserted_at (and rename it to getNewestInsertedAt or update usages) so the function returns the newest inserted_at string, and update all places that compute tailCursor (the tailCursor assignment sites and any helper used at the top and the two other occurrences noted) to use that inserted_at-based helper; ensure comparisons and types remain the same (string|undefined) and keep the same semantics for selecting the max timestamp.
96-114:⚠️ Potential issue | 🟡 MinorGuard optional config fields before constructing
config.The
!operators only suppress the type checker here. If the API returns a partial row, this mapping still builds aconfigobject withundefinedmembers.Suggested fix
+ const withConfig = (change: ConfigChange) => + change.config_id && change.type && change.name + ? [ + { + ...change, + config: { + id: change.config_id, + type: change.type, + name: change.name, + deleted_at: change.deleted_at + } + } + ] + : []; + - const baseChanges = (data?.changes ?? []).map((change) => ({ - ...change, - config: { - id: change.config_id!, - type: change.type!, - name: change.name!, - deleted_at: change.deleted_at - } - })); + const baseChanges = (data?.changes ?? []).flatMap(withConfig); - const tailedWithConfig = tailedChanges.map((change) => ({ - ...change, - config: { - id: change.config_id!, - type: change.type!, - name: change.name!, - deleted_at: change.deleted_at - } - })); + const tailedWithConfig = tailedChanges.flatMap(withConfig);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/ConfigChangesPage.tsx` around lines 96 - 114, The mapping that builds config inside baseChanges and tailedWithConfig uses non-null assertions (change.config_id!, change.type!, change.name!) which can create config objects with undefined values if the API returns partial rows; update the logic in the baseChanges and tailedWithConfig transformations to guard those optional fields—check that change.config_id, change.type, and change.name are present before constructing the config object (e.g., only include config when all required fields exist, or provide safe defaults), and ensure any downstream code handles the case where config may be undefined.
64-70:⚠️ Potential issue | 🟠 MajorReset tail state when the query scope changes.
This effect only clears the tail when Live turns off. While it stays on, changing filters, sort, or page reuses the old cursor and tailed rows for a different query.
Suggested fix
+ const tailScopeKey = params.toString(); + useEffect(() => { - if (!liveTail) { - setTailedChanges([]); - setTailCursor(undefined); - } - }, [liveTail]); + setTailedChanges([]); + setTailCursor(undefined); + }, [liveTail, tailScopeKey]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/ConfigChangesPage.tsx` around lines 64 - 70, The useEffect that currently resets tail state only when liveTail changes should also run when query scope changes so stale cursor/tailed rows aren't reused; update the effect tied to liveTail to include all query-scoping state (e.g., filters, sort, page, search/query variables) in its dependency array and call setTailedChanges([]) and setTailCursor(undefined) whenever any of those change while liveTail is true so the tail state is cleared for a new query; locate the effect by the useEffect that references liveTail, setTailedChanges, and setTailCursor.src/pages/config/details/ConfigDetailsChangesPage.tsx (3)
80-96:⚠️ Potential issue | 🟡 MinorGuard optional config fields before constructing
config.The non-null assertions only silence TypeScript here. If a delta row omits
config_id,type, orname, this still forwardsundefinedinto the table path.Suggested fix
+ const withConfig = (change: ConfigChange) => + change.config_id && change.type && change.name + ? [ + { + ...change, + config: { + id: change.config_id, + type: change.type, + name: change.name + } + } + ] + : []; + - const baseChanges = (data?.changes ?? []).map((change) => ({ - ...change, - config: { - id: change.config_id!, - type: change.type!, - name: change.name! - } - })); + const baseChanges = (data?.changes ?? []).flatMap(withConfig); - const tailedWithConfig = tailedChanges.map((change) => ({ - ...change, - config: { - id: change.config_id!, - type: change.type!, - name: change.name! - } - })); + const tailedWithConfig = tailedChanges.flatMap(withConfig);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 80 - 96, The mapping for baseChanges and tailedWithConfig blindly uses non-null assertions on change.config_id, change.type, and change.name; instead, guard those optional fields: either filter out changes missing required fields before mapping or build the config object conditionally (e.g., only include config when change.config_id && change.type && change.name), and provide safe fallbacks or omit the config property so undefined values aren't forwarded into table paths; update the logic in the baseChanges and tailedWithConfig transformations to perform this presence check and handle missing fields consistently.
11-19:⚠️ Potential issue | 🟠 MajorAdvance the live cursor on
inserted_at, notcreated_at.The tail cursor is still seeded and advanced from
created_at, but this feature is supposed to fetch deltas byinserted_at. A row inserted after the cursor with an oldercreated_atwill be missed.Suggested fix
-function getNewestCreatedAt(changes: ConfigChange[]): string | undefined { +function getNewestInsertedAt(changes: ConfigChange[]): string | undefined { let latest: string | undefined; for (const c of changes) { - const ts = typeof c.created_at === "string" ? c.created_at : undefined; + const ts = typeof c.inserted_at === "string" ? c.inserted_at : undefined; if (ts && (!latest || ts > latest)) { latest = ts; } } return latest; } @@ - setTailCursor(getNewestCreatedAt(data.changes)); + setTailCursor(getNewestInsertedAt(data.changes)); @@ - const newest = getNewestCreatedAt(incoming); + const newest = getNewestInsertedAt(incoming);Also applies to: 43-45, 67-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 11 - 19, The code seeds/advances the tail cursor using created_at (see getNewestCreatedAt), which misses rows inserted later that have older created_at; change getNewestCreatedAt to read and compare c.inserted_at (e.g., typeof c.inserted_at === "string") and return the newest inserted_at string, and update any other places in this file that reference created_at for cursor seeding/advancing (the other two similar blocks) to use inserted_at instead so the delta fetch uses the insertion timestamp.
48-54:⚠️ Potential issue | 🟠 MajorReset tail state when the query scope changes.
While Live stays on, changing filters, sort, page, or
idcarries the old cursor and tailed rows into the new query scope.Suggested fix
+ const tailScopeKey = `${id ?? ""}|${params.toString()}`; + useEffect(() => { - if (!liveTail) { - setTailedChanges([]); - setTailCursor(undefined); - } - }, [liveTail]); + setTailedChanges([]); + setTailCursor(undefined); + }, [liveTail, tailScopeKey]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 48 - 54, The effect that resets tail state only runs when liveTail toggles; update it so changing the query scope also clears the tail. Modify the useEffect that references liveTail to additionally depend on the query-scope variables (e.g., filters, sort, page, id or whatever props/state represent the current query) and, when liveTail is true and any of those scope values change, call setTailedChanges([]) and setTailCursor(undefined) (keep the existing liveTail check so it still clears when liveTail becomes false). Ensure you reference the existing identifiers liveTail, setTailedChanges, and setTailCursor in the updated effect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx`:
- Around line 41-45: The current useEffect seeds tailCursor from the loaded
slice (data.changes) which prevents live polling when the slice is empty or
causes replay of old rows; change the logic in the useEffect that watches
liveTail/data/changing tailCursor (and the similar block at lines 56-60) to
request a server-derived head/timestamp for the current filters when enabling
Live instead of (or if) data.changes is empty: call a backend helper (e.g.,
fetchHeadCursorForFilters or getServerNow) and setTailCursor with that server
head when data.changes.length === 0, otherwise keep the existing
getNewestCreatedAt(data.changes) behavior; ensure the network call runs only
when liveTail becomes true and tailCursor is unset so polling starts correctly.
---
Duplicate comments:
In `@src/pages/config/ConfigChangesPage.tsx`:
- Around line 57-60: The current useEffect bootstraps tailCursor from the
currently loaded slice (data.changes) which prevents Live from starting when the
slice is empty or causes replay of older rows; change the initialization in the
useEffect that checks liveTail to prefer a server-derived head (for the active
filters) and only fall back to getNewestCreatedAt(data.changes) when that head
is unavailable, and if there are no loaded rows use a server-side “now”
timestamp to seed tailCursor before enabling polling; update both occurrences
that set tailCursor (the useEffect around liveTail and the duplicate block at
the later lines) to call the server endpoint/prop for the head or now and then
call setTailCursor with that value.
- Around line 21-29: The code currently derives the live tail cursor from
created_at (function getNewestCreatedAt and any code that sets tailCursor) which
skips late-inserted rows; change getNewestCreatedAt to read and compare
c.inserted_at (and rename it to getNewestInsertedAt or update usages) so the
function returns the newest inserted_at string, and update all places that
compute tailCursor (the tailCursor assignment sites and any helper used at the
top and the two other occurrences noted) to use that inserted_at-based helper;
ensure comparisons and types remain the same (string|undefined) and keep the
same semantics for selecting the max timestamp.
- Around line 96-114: The mapping that builds config inside baseChanges and
tailedWithConfig uses non-null assertions (change.config_id!, change.type!,
change.name!) which can create config objects with undefined values if the API
returns partial rows; update the logic in the baseChanges and tailedWithConfig
transformations to guard those optional fields—check that change.config_id,
change.type, and change.name are present before constructing the config object
(e.g., only include config when all required fields exist, or provide safe
defaults), and ensure any downstream code handles the case where config may be
undefined.
- Around line 64-70: The useEffect that currently resets tail state only when
liveTail changes should also run when query scope changes so stale cursor/tailed
rows aren't reused; update the effect tied to liveTail to include all
query-scoping state (e.g., filters, sort, page, search/query variables) in its
dependency array and call setTailedChanges([]) and setTailCursor(undefined)
whenever any of those change while liveTail is true so the tail state is cleared
for a new query; locate the effect by the useEffect that references liveTail,
setTailedChanges, and setTailCursor.
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx`:
- Around line 80-96: The mapping for baseChanges and tailedWithConfig blindly
uses non-null assertions on change.config_id, change.type, and change.name;
instead, guard those optional fields: either filter out changes missing required
fields before mapping or build the config object conditionally (e.g., only
include config when change.config_id && change.type && change.name), and provide
safe fallbacks or omit the config property so undefined values aren't forwarded
into table paths; update the logic in the baseChanges and tailedWithConfig
transformations to perform this presence check and handle missing fields
consistently.
- Around line 11-19: The code seeds/advances the tail cursor using created_at
(see getNewestCreatedAt), which misses rows inserted later that have older
created_at; change getNewestCreatedAt to read and compare c.inserted_at (e.g.,
typeof c.inserted_at === "string") and return the newest inserted_at string, and
update any other places in this file that reference created_at for cursor
seeding/advancing (the other two similar blocks) to use inserted_at instead so
the delta fetch uses the insertion timestamp.
- Around line 48-54: The effect that resets tail state only runs when liveTail
toggles; update it so changing the query scope also clears the tail. Modify the
useEffect that references liveTail to additionally depend on the query-scope
variables (e.g., filters, sort, page, id or whatever props/state represent the
current query) and, when liveTail is true and any of those scope values change,
call setTailedChanges([]) and setTailCursor(undefined) (keep the existing
liveTail check so it still clears when liveTail becomes false). Ensure you
reference the existing identifiers liveTail, setTailedChanges, and setTailCursor
in the updated effect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1cc6e9b1-607b-4b05-91b1-094b866dae2c
📒 Files selected for processing (2)
src/pages/config/ConfigChangesPage.tsxsrc/pages/config/details/ConfigDetailsChangesPage.tsx
Comment on lines
+41
to
+45
| // Initialize cursor from base data when live tail is turned on | ||
| useEffect(() => { | ||
| if (liveTail && data?.changes?.length && !tailCursor) { | ||
| setTailCursor(getNewestCreatedAt(data.changes)); | ||
| } |
Contributor
There was a problem hiding this comment.
Bootstrapping from data.changes breaks Live mode.
This only seeds from the currently loaded slice, and the poll query will not run until tailCursor is set. That means enabling Live on an empty result never starts polling, and enabling it on an older slice can replay already-existing rows as “new”. Seed from a server-derived head for the current filters, or at least from a server-side “now” when there are no loaded rows.
Also applies to: 56-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/config/details/ConfigDetailsChangesPage.tsx` around lines 41 - 45,
The current useEffect seeds tailCursor from the loaded slice (data.changes)
which prevents live polling when the slice is empty or causes replay of old
rows; change the logic in the useEffect that watches liveTail/data/changing
tailCursor (and the similar block at lines 56-60) to request a server-derived
head/timestamp for the current filters when enabling Live instead of (or if)
data.changes is empty: call a backend helper (e.g., fetchHeadCursorForFilters or
getServerNow) and setTailCursor with that server head when data.changes.length
=== 0, otherwise keep the existing getNewestCreatedAt(data.changes) behavior;
ensure the network call runs only when liveTail becomes true and tailCursor is
unset so polling starts correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2925
Summary by CodeRabbit
New Features
Chores