fix: skills repo removes hydration and polishes the whole flow - #4445
Merged
akshaydeo merged 1 commit intoJun 16, 2026
Merged
Conversation
|
|
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
18 tasks
Contributor
|
Warning Review limit reached
More reviews will be available in 23 minutes and 35 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughRemoves server-side inline text hydration for skill files ( ChangesBackend: Remove InlineContent Hydration
Skills Repo UI: Form Helpers & Metadata
Skills Repo UI: Read-Only Shared Components
Skills Repo UI: File Preview & Editing
Skills Repo UI: File Manager
Skills Repo UI: Edit Form Refactor
Skills Repo UI: Version & Layout Updates
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over User,SkillEditView: Edit Pane Selection Flow
User->>SkillEditView: clicks Details / Metadata / Extra Frontmatter tab
SkillEditView->>SkillEditView: sets selectedDetailsPane
SkillEditView->>DetailsEditorPane: renders (if details selected)
DetailsEditorPane->>DetailsEditorPane: wire form.setDescription, form.validateField
SkillEditView->>MetadataEditorPane: renders (if metadata selected)
MetadataEditorPane->>MetadataTableEditor: display and edit metadata
SkillEditView->>ExtraFrontmatterEditorPane: renders (if extra_frontmatter selected)
ExtraFrontmatterEditorPane->>CodeEditor: display and edit JSON
end
rect rgba(144, 238, 144, 0.5)
Note over User,ServeEndpoint: File Source Editing Flow
User->>FileManagerView: clicks file in edit mode
FileManagerView->>FilePreviewPane: pass file, onFileUpdate
FilePreviewPane->>FilePreview: mode=edit
FilePreview->>FileSourceEditor: source_type, file
alt Text (saved)
FileSourceEditor->>ServeEndpoint: fetch serve content
ServeEndpoint-->>FileSourceEditor: text payload
else URL
FileSourceEditor->>FileSourceEditor: render url Input
User->>FileSourceEditor: edit source_url
else DataURL
FileSourceEditor->>FileSourceEditor: render textarea or binary fallback
else Text (local)
FileSourceEditor->>FileSourceEditor: render textarea seeded from inlineText
end
FileSourceEditor-->>FilePreviewPane: emit onFileUpdate(updates)
end
rect rgba(255, 218, 185, 0.5)
Note over User,SkillReadOnlyContent: Read-Only Pane Selection
User->>SkillFilesSidebar: clicks Metadata / Extra Frontmatter / File
SkillFilesSidebar->>SkillReadOnlyContent: emit selection change
SkillReadOnlyContent->>Right-pane: render based on selectedPath
alt Metadata
Right-pane->>ReadOnlyMetadataTable: display
else Extra Frontmatter
Right-pane->>ReadOnlyYamlBlock: display
else File
Right-pane->>FilePreview: mode=view
else Nothing
Right-pane->>ReadOnlySkillBody: display rendered/raw tabs
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
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 unit tests (beta)
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: 6
🤖 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 `@ui/app/workspace/skills-repo/components/filePreview.tsx`:
- Around line 412-425: The needsFetch condition currently allows fetching and
text-decoding for all saved dataurl types including binary data, which can
corrupt image/binary content if edited. Add a MIME type check to the needsFetch
condition that restricts the text-decoding path to only text-like files (e.g.,
checking if the MIME type starts with "text/" or specific safe types like
"application/json"). For binary dataurl files, implement an alternative path
that preserves the raw data URL without decoding through res.text(), preventing
data corruption when users interact with the textarea.
In `@ui/app/workspace/skills-repo/components/shared.tsx`:
- Around line 714-759: The span element containing item.name has min-w-0 and
truncate classes applied, which prevents horizontal scrolling and keeps long
filenames ellipsized. To enable horizontal scrolling of full filenames as
intended by the sidebar's min-w-max wrapper, remove the min-w-0 and truncate
classes from the className of the span that displays item.name, while preserving
the other utility classes like flex-1, font-mono, text-xs, and the conditional
font-medium styling for folders.
In `@ui/app/workspace/skills-repo/components/skillCreatorView.tsx`:
- Line 34: The permission-denied state container in skillCreatorView.tsx is
missing a height or flex-grow anchor that is required for the flex centering
utilities to work properly. Add a height constraint (such as h-full) or
flex-grow property (such as flex-1) to the className of the div element that
contains "flex items-center justify-center" so that the centering utilities can
properly position the message vertically within the create-view pane.
In `@ui/app/workspace/skills-repo/components/skillDetailsView.tsx`:
- Around line 90-95: The useEffect hook that populates the form state is
resetting the form on every skill or highestVersion change, which wipes out
unsaved edits when isEditing is true. Add a guard condition inside the useEffect
to check that isEditing is false before calling form.reset(state) with the
buildFormState() result. This ensures the form is only reset during initial load
or when exiting edit mode, but preserves local form state while actively
editing.
In `@ui/app/workspace/skills-repo/components/skillListView.tsx`:
- Line 423: The className for the embedded empty state container in
skillListView is using min-h-screen which forces viewport-height behavior inside
a nested workspace pane, causing unwanted vertical scroll/overflow. Replace
min-h-screen with a parent-bound sizing option such as h-full, min-h-0, or
flex-1 to preserve predictable workspace UI behavior and prevent the empty-state
path from introducing extra scrolling.
In `@ui/app/workspace/skills-repo/forms/skillEditForm.tsx`:
- Around line 211-248: Add stable data-testid attributes to interactive pane
selector buttons across two files for E2E test compatibility. In
ui/app/workspace/skills-repo/forms/skillEditForm.tsx#L211-L248, add data-testid
props to the dynamically mapped buttons (use skill-details-pane-btn and
skill-metadata-pane-btn) and to the separate Extra Frontmatter button (use
skill-frontmatter-pane-btn). Similarly, in
ui/app/workspace/skills-repo/components/shared.tsx#L896-L924, add data-testid
attributes to the corresponding read-only Metadata and Extra Frontmatter
selector elements with appropriate test IDs for navigation purposes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c6631ddf-7864-4509-bf83-93aaf74fc05f
📒 Files selected for processing (15)
framework/configstore/skills.goui/app/workspace/skills-repo/components/fileManagerView.tsxui/app/workspace/skills-repo/components/filePreview.tsxui/app/workspace/skills-repo/components/helpers.tsui/app/workspace/skills-repo/components/metadataEditorTableView.tsxui/app/workspace/skills-repo/components/shared.tsxui/app/workspace/skills-repo/components/skillCreatorView.tsxui/app/workspace/skills-repo/components/skillDetailsView.tsxui/app/workspace/skills-repo/components/skillListView.tsxui/app/workspace/skills-repo/dialogs/skillVersionDialog.tsxui/app/workspace/skills-repo/dialogs/versionDetailsDialog.tsxui/app/workspace/skills-repo/forms/skillEditForm.tsxui/app/workspace/skills-repo/forms/skillEditFormFields.tsxui/app/workspace/skills-repo/page.tsxui/components/ui/treeView.tsx
💤 Files with no reviewable changes (1)
- framework/configstore/skills.go
akshaydeo
changed the base branch from
06-16-skill_ui_ux_fixes
to
graphite-base/4445
June 16, 2026 11:13
akshaydeo
force-pushed
the
graphite-base/4445
branch
from
June 16, 2026 11:31
48f38b8 to
55536ac
Compare
akshaydeo
force-pushed
the
06-16-fix_skills_repo_removes_hydration_and_polishes_the_whole_flow
branch
from
June 16, 2026 11:31
e462f05 to
c0f691e
Compare
akshaydeo
changed the base branch from
graphite-base/4445
to
06-16-skill_ui_ux_fixes
June 16, 2026 11:31
akshaydeo
force-pushed
the
06-16-fix_skills_repo_removes_hydration_and_polishes_the_whole_flow
branch
2 times, most recently
from
June 16, 2026 11:32
868bf95 to
f2784da
Compare
Contributor
Merge activity
|
akshaydeo
changed the base branch from
06-16-skill_ui_ux_fixes
to
graphite-base/4445
June 16, 2026 11:34
akshaydeo
force-pushed
the
06-16-fix_skills_repo_removes_hydration_and_polishes_the_whole_flow
branch
from
June 16, 2026 11:35
f2784da to
1568fa4
Compare
akshaydeo
deleted the
06-16-fix_skills_repo_removes_hydration_and_polishes_the_whole_flow
branch
June 16, 2026 11:36
Contributor
Confidence Score: 3/5I don't think this is safe to merge yet.
 T-Rex LogsWhat T-Rex did
Reviews (1): Last reviewed commit: "fix: skills repo removes hydration and p..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return next; | ||
| }); | ||
| }, []); | ||
| }; |
Contributor
There was a problem hiding this comment.
When search is active, this builds the tree from filteredFiles, and buildTree assigns each row an index from that filtered array. The row actions later pass that fileIndex to handlers that operate on the full files array. If the user searches for b.txt in [a.txt, b.txt], the visible b.txt row gets index 0, so select, rename, move, or delete can affect a.txt instead.
| size="icon" | ||
| className="text-muted-foreground h-6 w-6" | ||
| data-testid={`skill-file-actions-${index}`} | ||
| data-testid={`skill-file-actions-${basename(file.path)}`} |
Contributor
There was a problem hiding this comment.
This changes the file action test id from the previous index-based value to skill-file-actions-${basename(file.path)} without updating tests. Existing Playwright selectors like skill-file-actions-0 can no longer find the menu, and duplicate basenames in different folders now produce duplicate test ids. Please preserve the old test id, or add the new selector separately.
Suggested change
| data-testid={`skill-file-actions-${basename(file.path)}`} | |
| data-testid={`skill-file-actions-${index}`} |
Rule Used: UI changes must preserve data-testid attributes us... (source)
| aria-label={`Actions for ${item.name}`} | ||
| > | ||
| <Button variant="ghost" size="icon" className="h-6 w-6" aria-label={`Actions for ${item.name}`}> | ||
| <MoreHorizontal className="h-3.5 w-3.5" /> |
Contributor
There was a problem hiding this comment.
The read-only file tree action buttons lost their data-testids (skill-file-row-actions and skill-files-tree-actions) without matching test updates. Playwright tests that open the file-row or root tree menus will no longer be able to locate these controls. Please keep those attributes on the triggers.
Rule Used: UI changes must preserve data-testid attributes us... (source)
18 tasks
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
3 participants
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.
Summary
Removes server-side hydration of inline text content for skill files and applies
a comprehensive set of UI polish fixes across the entire Skills Repository flow,
addressing layout overflow issues, scroll behavior, tree view truncation, and
form state management.
Changes
hydrateInlineTextContentfromconfigstore/skills.go— inlinetext content no longer needs to be hydrated from DB blobs on the server side;
content is served via the file endpoint instead
page.tsx,skillDetailsView,skillCreatorView,skillListView) by replacinghardcoded viewport height calculations (
h-[calc(100dvh-1rem)]) with flexibleh-fulllayoutstreeView.tsx— changedmin-w-0 overflow-hiddentomin-w-maxso long file names and deep nestingare no longer clipped
skillDetailsView— replaceduseMemo-basedgetSkillFormStatewith a plainbuildFormState()functionand updated the
useEffectdependency to[skill, highestVersion]for morepredictable form resets
validateFieldinhelpers.ts— replacedswitchwithif/elsechain, removed unnecessaryuseCallbackwrappergetPayloadin the skill form hook — extracted JSON parsinginto explicit variables for readability
yamlMetadataFieldhelper and inlined its logic incomposeFrontmatterfileManagerView,filePreview,shared,skillEditForm,metadataEditorTableView, andversionDetailsDialogwith layout, spacing,and scroll fixes throughout
Type of change
Affected areas
How to test
without vertical overflow or extra scrollbars
correctly, tree view shows full file names without truncation
the original skill data
end-to-end
behavior
Screenshots/Recordings
N/A — layout and polish changes throughout the skills repo flow.
Breaking changes
Related issues
N/A
Security considerations
None. The removal of
hydrateInlineTextContentdoes not expose any new datapaths — file content continues to be served through the existing file endpoint.
Checklist
docs/contributing/README.mdand followed the guidelines