Modified Pixel for Python - #4271
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Pixelbot document and category administration APIs and UI, including uploads and editing. Extends RAG messages with editor context. Updates chatbot controls and floating positions. ChangesPixelbot document administration
RAG chatbot context and controls
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (7)
src/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsx (1)
186-199: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExpose the info popover state to assistive technology.
The button toggles
infoOpenbut reports no state. Addaria-expanded={infoOpen}so screen reader users learn that the button opens a disclosure.♻️ Proposed change
<button type="button" className={classes.infoButton} onClick={() => setInfoOpen(prev => !prev)} + aria-expanded={infoOpen} aria-label="About this screen" >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsx` around lines 186 - 199, Update the info toggle button in the PixelbotDocumentsPanel component to include aria-expanded={infoOpen}, keeping it synchronized with the existing state used to render the infoPopover.src/features/adminPanel/subcomponents/DocumentDetailPopup.tsx (2)
37-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
documentprop shadows the globaldocument.Inside this module,
documentrefers to the prop. Any later use of the DOM global, for exampledocument.addEventListeneras used inPixelbotDocumentsPanel.tsxat line 87, would silently resolve to the prop and throw at runtime.Rename the prop, for example to
pixelbotDocument.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/adminPanel/subcomponents/DocumentDetailPopup.tsx` around lines 37 - 45, Rename the document prop in DocumentDetailPopup and update all references, including Props and callers, to a non-conflicting name such as pixelbotDocument so DOM global document access remains unshadowed.
50-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why the effect depends only on
document?.id.CI reports
react-hooks/exhaustive-depsfor this effect. Addingdocumentto the array would reset the draft on every parent refresh and discard in-progress edits, becausePixelbotDocumentsPanelcreates a newdocumentsarray on eachrefresh(). The current dependency list is therefore intentional.Add a comment and an explicit disable so a later contributor does not "fix" the warning and break editing.
♻️ Proposed change
+ // Reset the draft only when a different document is shown. Depending on `document` itself + // would discard in-progress edits, because the parent rebuilds the documents array on every + // refresh. + // eslint-disable-next-line react-hooks/exhaustive-deps useEffect(() => { if (document) { setDraft(draftFrom(document)); setMode(initialMode); } }, [document?.id]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/adminPanel/subcomponents/DocumentDetailPopup.tsx` around lines 50 - 55, Add an inline comment above the useEffect dependency array explaining that it intentionally depends only on document?.id to avoid resetting in-progress edits when parent refreshes recreate the document object, and add a narrowly scoped react-hooks/exhaustive-deps disable for this effect. Keep the existing effect behavior unchanged.Source: Pipeline failures
src/features/adminPanel/subcomponents/AddDocumentsModal.tsx (1)
151-157: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the nested
setSelectedFileIdout of thesetBatchupdater.React may invoke a state updater more than once, and StrictMode does so deliberately. Queueing another state update inside the updater is not pure. The current call is idempotent, so there is no visible defect today, but the pattern breaks as soon as the logic gains a non-idempotent step.
Compute the next batch outside the setter.
♻️ Proposed change
- const removeFile = useCallback((id: string) => { - setBatch(prev => { - const next = prev.filter(f => f.id !== id); - setSelectedFileId(sel => (sel === id ? (next[0]?.id ?? null) : sel)); - return next; - }); - }, []); + const removeFile = useCallback( + (id: string) => { + const next = batch.filter(f => f.id !== id); + setBatch(next); + if (selectedFileId === id) { + setSelectedFileId(next[0]?.id ?? null); + } + }, + [batch, selectedFileId], + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/adminPanel/subcomponents/AddDocumentsModal.tsx` around lines 151 - 157, Update removeFile to compute the filtered next batch outside the setBatch updater, then update selectedFileId based on that result and return the next batch from a pure setBatch callback. Preserve the existing fallback to the first remaining file ID or null when the removed file was selected.Source: Linters/SAST tools
src/features/adminPanel/subcomponents/DocumentDirectory.module.css (1)
202-216: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSeveral class blocks have no consumer.
.docHeaderRow,.docHeaderLabel,.emptyLink(lines 274-284),.progressTrackand.progressFill(lines 470-481), and.popupEyebrow(lines 728-734) are not referenced byPixelbotDocumentsPanel.tsx,AddDocumentsModal.tsx, orDocumentDetailPopup.tsx.
.docHeaderRowand.docHeaderLabeldescribe column headers for the four-column.docRowgrid. The panel renders.docRowwith no header row, so the Date, Status, and Actions columns have no labels. Confirm whether the header markup was dropped by mistake. Remove the remaining blocks if they are leftovers.#!/bin/bash # Check for consumers of the CSS module classes that appear unused. for cls in docHeaderRow docHeaderLabel emptyLink progressTrack progressFill popupEyebrow; do echo "== $cls ==" rg -n --iglob '*.tsx' --iglob '*.ts' -- "$cls" done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/adminPanel/subcomponents/DocumentDirectory.module.css` around lines 202 - 216, Verify whether the document panel’s four-column header markup was accidentally omitted; if so, add consumers for docHeaderRow and docHeaderLabel matching the docRow columns. Otherwise remove the unused CSS blocks docHeaderRow, docHeaderLabel, emptyLink, progressTrack, progressFill, and popupEyebrow from the stylesheet.src/commons/sagas/RequestsSaga.ts (2)
1485-1485: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
unknown[]overany[]for the map preview.The only consumer,
handleOpenMapPreviewinsrc/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsx, passes the value toJSON.stringify.unknown[]satisfies that use and stopsanyfrom spreading into callers.♻️ Proposed change
-export const getPixelbotDocumentMapPreview = async (tokens: Tokens): Promise<any[] | null> => { +export const getPixelbotDocumentMapPreview = async (tokens: Tokens): Promise<unknown[] | null> => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commons/sagas/RequestsSaga.ts` at line 1485, Update the return type of getPixelbotDocumentMapPreview from any[] | null to unknown[] | null, preserving its existing behavior and ensuring callers such as handleOpenMapPreview can still pass the result to JSON.stringify.
1548-1563: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
renamePixelbotDocumenthelper.
renamePixelbotDocumentis defined insrc/commons/sagas/RequestsSaga.ts, but no tracked file calls it. Remove it unless the document-rename UI is added in this PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commons/sagas/RequestsSaga.ts` around lines 1548 - 1563, Remove the unused renamePixelbotDocument helper from RequestsSaga.ts, including its request logic, since no tracked code calls it; do not add replacement behavior unless a document-rename UI is introduced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/adminPanel/subcomponents/AddDocumentsModal.tsx`:
- Line 22: Replace the UTC-based todayIso calculation in
AddDocumentsModal.tsx#L22-L22 with local getFullYear, getMonth, and getDate
formatting so immediate releases use the admin’s local date. In
PixelbotDocumentsTypes.ts#L71-L77, update pixelbotDocumentStatus to use the same
local-date helper; both sites must produce matching yyyy-mm-dd values.
- Around line 185-203: Update handleSaveAll in
src/features/adminPanel/subcomponents/AddDocumentsModal.tsx (lines 185-203) to
wrap the save flow in try/finally and reset isSaving in finally; also reset
isSaving in handleClose. Update the save handler in
src/features/adminPanel/subcomponents/DocumentDetailPopup.tsx (lines 84-103)
similarly, ensuring isSaving resets whether the save succeeds or throws.
- Around line 380-396: Associate every form label in AddDocumentsModal and
DocumentDetailPopup with its corresponding field by assigning each
input/select/textarea a unique id and matching the label’s htmlFor. Apply this
consistently to the Title, Summary, Category, and Release date fields,
preserving the existing field behavior and values.
- Around line 67-103: Update runUpload to catch rejected uploadPixelbotDocuments
calls and mark every file in the current batch as phase 'error' with a retryable
errorMessage, ensuring setBatch runs when the request or response parsing fails.
Also validate that entries contains one result per file before applying
index-based metadata, treating a short or missing response as a batch failure
rather than silently mismatching files.
- Line 133: Update onDropRejected in AddDocumentsModal to collect the rejected
filenames and display them with showDangerMessage, importing that helper from
the notifications module. Include the accepted file types in the message so
admins understand why the drop was rejected.
In `@src/features/adminPanel/subcomponents/DocumentDetailPopup.tsx`:
- Around line 240-242: Allow saving documents with a null releaseDate by
removing the releaseDate requirement from the Save button’s intent and disabled
conditions in DocumentDetailPopup. Keep saving gated by dirty and isSaving,
preserving the documented undated-document behavior.
In `@src/features/adminPanel/subcomponents/DocumentDirectory.module.css`:
- Around line 1-23: Move the --dd-* custom property declarations from .directory
to :root in DocumentDirectory.module.css so Blueprint dialog portals can resolve
them. In AddDocumentsModal.tsx at lines 249-266, verify .dropzone, .formInput,
.formLabel, and .formSelect use the intended borders and text colors after the
scope fix; DocumentDetailPopup.tsx requires no direct change because it shares
these classes.
In `@src/features/adminPanel/subcomponents/PixelbotConfigPanel.tsx`:
- Around line 31-33: Update the routing prompt containing the document-selection
instructions so the date-based guidance follows an actual current-date injection
into the prompt, alongside the dynamic %DOCUMENT_MAP% content. If the
surrounding prompt-building flow cannot provide today’s date, remove the
date-dependent instruction instead of leaving the model to rely on unavailable
context.
In `@src/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsx`:
- Around line 238-241: The category rename onKeyDown handler at
src/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsx:238-241 must
return immediately when e.nativeEvent.isComposing is true before handling Enter
or Escape; apply the same first-statement guard to the new-category onKeyDown
handler at
src/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsx:324-330.
- Around line 272-308: Run the project formatter on all three affected files. In
src/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsx lines 272-308,
reformat the over-indented docs.map document-row block and wrap the over-width
lines 202 and 402; in
src/features/adminPanel/subcomponents/AddDocumentsModal.tsx lines 205-207,
reformat the categoryOptions memo arguments and wrap lines 9, 162, 351, and 401;
in src/features/adminPanel/subcomponents/DocumentDetailPopup.tsx lines 68-70,
reformat the categoryOptions memo arguments and wrap lines 141 and 172.
- Around line 226-229: Make both interactive containers keyboard accessible: in
src/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsx lines 226-229,
update the category row around toggleCategoryOpen to add role="button",
tabIndex={0}, aria-expanded={isOpen}, and an onKeyDown handler that toggles on
Enter or Space; in src/features/adminPanel/subcomponents/AddDocumentsModal.tsx
lines 276-284, update the batch file item to add role="button", tabIndex={0},
and an onKeyDown handler that performs the same file-selection/edit action on
Enter or Space.
- Around line 173-177: Update handleOpenMapPreview to clear the existing
mapPreview immediately before starting getPixelbotDocumentMapPreview, while
preserving the current loading and response handling behavior.
- Around line 270-271: Update the expanded-category rendering condition in
PixelbotDocumentsPanel so the category body renders whenever isOpen is true,
including when docs is empty. Within that body, display the existing emptyRow
style and an appropriate empty-state message for categories with no documents,
while preserving the current document rendering for non-empty categories.
In `@src/features/adminPanel/subcomponents/PixelbotDocumentsTypes.ts`:
- Around line 71-77: Update pixelbotDocumentStatus to derive today from the
local Date components rather than new Date().toISOString(), formatting the local
year, month, and day as yyyy-mm-dd before comparing with releaseDate. Preserve
the existing Live behavior for null dates and the current comparison semantics.
---
Nitpick comments:
In `@src/commons/sagas/RequestsSaga.ts`:
- Line 1485: Update the return type of getPixelbotDocumentMapPreview from any[]
| null to unknown[] | null, preserving its existing behavior and ensuring
callers such as handleOpenMapPreview can still pass the result to
JSON.stringify.
- Around line 1548-1563: Remove the unused renamePixelbotDocument helper from
RequestsSaga.ts, including its request logic, since no tracked code calls it; do
not add replacement behavior unless a document-rename UI is introduced.
In `@src/features/adminPanel/subcomponents/AddDocumentsModal.tsx`:
- Around line 151-157: Update removeFile to compute the filtered next batch
outside the setBatch updater, then update selectedFileId based on that result
and return the next batch from a pure setBatch callback. Preserve the existing
fallback to the first remaining file ID or null when the removed file was
selected.
In `@src/features/adminPanel/subcomponents/DocumentDetailPopup.tsx`:
- Around line 37-45: Rename the document prop in DocumentDetailPopup and update
all references, including Props and callers, to a non-conflicting name such as
pixelbotDocument so DOM global document access remains unshadowed.
- Around line 50-55: Add an inline comment above the useEffect dependency array
explaining that it intentionally depends only on document?.id to avoid resetting
in-progress edits when parent refreshes recreate the document object, and add a
narrowly scoped react-hooks/exhaustive-deps disable for this effect. Keep the
existing effect behavior unchanged.
In `@src/features/adminPanel/subcomponents/DocumentDirectory.module.css`:
- Around line 202-216: Verify whether the document panel’s four-column header
markup was accidentally omitted; if so, add consumers for docHeaderRow and
docHeaderLabel matching the docRow columns. Otherwise remove the unused CSS
blocks docHeaderRow, docHeaderLabel, emptyLink, progressTrack, progressFill, and
popupEyebrow from the stylesheet.
In `@src/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsx`:
- Around line 186-199: Update the info toggle button in the
PixelbotDocumentsPanel component to include aria-expanded={infoOpen}, keeping it
synchronized with the existing state used to render the infoPopover.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ab196c8f-db63-4cee-a60e-a1a570005236
📒 Files selected for processing (12)
src/commons/sagas/RequestsSaga.tssrc/components/ui/chatbot/ChatBox.tsxsrc/components/ui/chatbot/FloatingChatbot.tsxsrc/components/ui/chatbot/FloatingChatbotButton.tsxsrc/features/adminPanel/subcomponents/AddDocumentsModal.tsxsrc/features/adminPanel/subcomponents/DocumentDetailPopup.tsxsrc/features/adminPanel/subcomponents/DocumentDirectory.module.csssrc/features/adminPanel/subcomponents/PixelbotConfigPanel.tsxsrc/features/adminPanel/subcomponents/PixelbotDocumentsPanel.tsxsrc/features/adminPanel/subcomponents/PixelbotDocumentsTypes.tssrc/features/ragChat/api.tssrc/pages/academy/ragChatbot/RagChatbot.tsx
💤 Files with no reviewable changes (1)
- src/components/ui/chatbot/ChatBox.tsx
Coverage Report for CI Build 31365312937Coverage decreased (-0.02%) to 45.358%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
sayomaki
left a comment
There was a problem hiding this comment.
Overall the changes are good, but there's a few things that I would like to point out and/or ask about.
sayomaki
left a comment
There was a problem hiding this comment.
LGTM, but do remember to update the category_id changes on the backend! Will leave a comment there.
Description
This PR adds 2 main features for pixel
Type of change
How to test
Checklist