Skip to content

feat(intelligent-assistant): add inline rename for notebook resources#3834

Open
its-mitesh-kumar wants to merge 2 commits into
redhat-developer:mainfrom
its-mitesh-kumar:feat/intelligent-assistant-inline-document-rename
Open

feat(intelligent-assistant): add inline rename for notebook resources#3834
its-mitesh-kumar wants to merge 2 commits into
redhat-developer:mainfrom
its-mitesh-kumar:feat/intelligent-assistant-inline-document-rename

Conversation

@its-mitesh-kumar

@its-mitesh-kumar its-mitesh-kumar commented Jul 19, 2026

Copy link
Copy Markdown
Member

Description

Adds inline rename capability for notebook documents (resources) in the DocumentSidebar. Users can double-click a filename or select "Rename" from the kebab menu to edit the document name inline using a PatternFly TextInput. The file extension is shown as a static suffix and cannot be accidentally modified. Frontend conflict validation prevents renaming to an existing document name with real-time error feedback. A new backend PATCH endpoint handles the rename with rollback protection if the vector store re-creation fails.

UI after changes

S_.2026-07-19.at.3.04.32.PM.mov
S_.2026-07-19.at.3.05.43.PM.mov

Fixed

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@rhdh-gh-app

rhdh-gh-app Bot commented Jul 19, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-intelligent-assistant-backend workspaces/intelligent-assistant/plugins/intelligent-assistant-backend minor v3.0.3
@red-hat-developer-hub/backstage-plugin-intelligent-assistant workspaces/intelligent-assistant/plugins/intelligent-assistant minor v3.0.3

@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.44828% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.29%. Comparing base (8d3503a) to head (6f566cd).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3834      +/-   ##
==========================================
+ Coverage   56.26%   56.29%   +0.03%     
==========================================
  Files        2499     2500       +1     
  Lines       96058    96173     +115     
  Branches    26644    26665      +21     
==========================================
+ Hits        54047    54141      +94     
- Misses      41683    41704      +21     
  Partials      328      328              
Flag Coverage Δ *Carryforward flag
adoption-insights 83.70% <ø> (ø) Carriedforward from 0e86cae
ai-integrations 69.00% <ø> (ø) Carriedforward from 0e86cae
app-defaults 69.79% <ø> (ø) Carriedforward from 0e86cae
augment 46.39% <ø> (ø) Carriedforward from 0e86cae
boost 73.13% <ø> (ø) Carriedforward from 0e86cae
bulk-import 72.46% <ø> (ø) Carriedforward from 0e86cae
cost-management 14.10% <ø> (ø) Carriedforward from 0e86cae
dcm 61.81% <ø> (ø) Carriedforward from 0e86cae
extensions 61.53% <ø> (ø) Carriedforward from 0e86cae
global-floating-action-button 71.18% <ø> (ø) Carriedforward from 0e86cae
global-header 59.71% <ø> (ø) Carriedforward from 0e86cae
homepage 50.23% <ø> (ø) Carriedforward from 0e86cae
install-dynamic-plugins 56.77% <ø> (ø) Carriedforward from 0e86cae
intelligent-assistant 69.27% <78.44%> (+0.25%) ⬆️
konflux 91.49% <ø> (ø) Carriedforward from 0e86cae
lightspeed 69.02% <ø> (ø) Carriedforward from 0e86cae
mcp-integrations 85.46% <ø> (ø) Carriedforward from 0e86cae
orchestrator 53.74% <ø> (ø) Carriedforward from 0e86cae
quickstart 65.63% <ø> (ø) Carriedforward from 0e86cae
sandbox 79.56% <ø> (ø) Carriedforward from 0e86cae
scorecard 82.96% <ø> (ø) Carriedforward from 0e86cae
theme 61.26% <ø> (ø) Carriedforward from 0e86cae
translations 7.25% <ø> (ø) Carriedforward from 0e86cae
x2a 78.68% <ø> (ø) Carriedforward from 0e86cae

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d3503a...6f566cd. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@its-mitesh-kumar

Copy link
Copy Markdown
Member Author

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 19, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:19 AM UTC · Completed 10:26 AM UTC
Commit: 8d3503a · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review — approve

PR: #3834 — feat(intelligent-assistant): add inline rename for notebook resources
Scope: workspaces/intelligent-assistant (frontend plugin + backend plugin)

Summary

This PR adds inline rename functionality for notebook document resources. The implementation is well-structured, spanning backend (PATCH endpoint with conflict detection and rollback) and frontend (inline TextInput with extension separation, conflict validation, keyboard handling). Test coverage is thorough across all layers.

What looks good

  • Backend endpoint design — The PATCH route at /v1/sessions/:sessionId/documents/:documentId correctly uses generalRateLimiter, requireNotebooksPermission, and requireSessionOwnership(), matching the security posture of adjacent endpoints (DELETE, GET status). Input validation catches empty/whitespace titles with a 400 response.
  • Rollback logicdocumentService.renameDocument handles the case where the vector store re-create fails after delete by attempting to restore the original entry. Error logging distinguishes between rollback success and orphaned-document failure, aiding debuggability.
  • Frontend conflict validation — The getConflictError callback provides real-time client-side conflict detection against the current document list, preventing unnecessary round-trips. The backend also validates conflicts independently (defense in depth).
  • Extension handlingsplitFileName correctly handles dotfiles (lastDot <= 0 treats names like .env as extensionless), and the extension is rendered as a static suffix outside the input, preventing accidental modification.
  • Test coverage — Backend tests cover happy path, 404, 409, and empty-title validation. Frontend tests cover double-click entry, Enter to save, Escape to cancel, unchanged-name skip, empty-name skip, kebab menu trigger, tooltip presence, conflict error display, and conflict-free state.
  • i18n — All five translation files (de, es, fr, it, ja) updated alongside the reference strings, with proper interpolation templates.

Findings

1. Non-atomic rename relies on delete-then-create with a brief inconsistency window

Severity low
Category correctness
File workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/notebooks/documents/documentService.ts
Line ~290 (in diff: the renameDocument method)

renameDocument deletes the old vector store file entry and then re-creates it with the new title. Between these two operations there is a brief window where the document does not exist in the vector store. A concurrent /query request hitting this window would miss the document's content. The rollback handles the most likely failure mode (create throws), but a process crash between delete and create would leave the document absent with the underlying file orphaned. This is an inherent limitation of the vector store API (no in-place attribute update), and the developer has mitigated it as well as is practical. Worth noting for operational awareness; no code change needed.

2. No maximum length or character validation on new title

Severity low
Category correctness
File workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/notebooks/notebooksRouters.ts
Line ~453 (in diff: the PATCH handler)

The PATCH endpoint validates that title is a non-empty, non-whitespace string, but does not enforce a maximum length or restrict characters (e.g., path separators, control characters). Extremely long titles could cause truncation or display issues in the sidebar, and special characters might interact poorly with the vector store API's attribute storage. Consider adding a length cap consistent with other title fields in the codebase.

3. Rename tooltip and kebab item shown unconditionally

Severity low
Category style/conventions
File workspaces/intelligent-assistant/plugins/intelligent-assistant/src/components/notebooks/DocumentSidebar.tsx
Line ~376 (in diff: Typography with tooltip)

The "Double-click to rename" tooltip and the "Rename" kebab dropdown item are always rendered, regardless of whether onRenameDocument is provided. If DocumentSidebar is ever used without the callback, users would see the rename UI but saves would silently no-op (via optional chaining onRenameDocument?.(...)). This mirrors the existing pattern for onDeleteDocument, so it's consistent. No change required unless the component is reused elsewhere without rename support.

Verdict

The implementation is solid, well-tested, and follows established patterns in the codebase. The non-atomic rename is an inherent API limitation handled with appropriate rollback protection. The PR is safe to merge.


Labels: PR adds a PATCH endpoint and inline rename UI within the intelligent-assistant workspace

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jul 19, 2026
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
@sonarqubecloud

Copy link
Copy Markdown

@its-mitesh-kumar

Copy link
Copy Markdown
Member Author

@JslYoon Please review the backend changes.

@HusneShabbir HusneShabbir left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I noticed a slight delay in the file renaming process. It seems the non-atomic rename relies on a delete-then-create flow, which introduces a brief inconsistency window. I'd encourage @JslYoon to take a closer look and suggest any improvements if applicable.

Also, could you please address comments 1 and 2 from the Fullsend review? Specifically:

  • 1: The non-atomic rename (deletecreate) introduces a brief inconsistency window.
  • 2: Very long title names don't add much value; it would be better to consider adding a length cap.

#3834 (comment)

Screen.Recording.2026-07-20.at.1.24.16.PM.mov

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

Labels

ready-for-merge All reviewers approved — ready to merge workspace/intelligent-assistant

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants