feat(docs): turn documents into an operations wiki - #2542
Merged
Conversation
📝 WalkthroughWalkthroughThis change adds document tags, wiki links, backlinks, revisions, attachments, templates, live DAG blocks, and document navigation. Git sync classifies document assets as binary items and reports size metadata. ChangesDocument platform and persistence
Document asset synchronization
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (15)
ui/src/pages/git-sync/DiffModal.tsx (1)
108-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a compact binary information block.
Reduce
p-6to compact padding such asp-3. Applybg-muted/30to the information block. This keeps the modal information-dense and preserves semantic theme styling.As per coding guidelines, “Use information-dense, compact UI design with minimal whitespace.” Based on learnings, “use the semantic
bg-muted/30theme token for information blocks.”🤖 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 `@ui/src/pages/git-sync/DiffModal.tsx` around lines 108 - 137, Update the binary attachment information block in the binary rendering branch to use compact padding such as p-3 instead of p-6, and add the semantic bg-muted/30 background class while preserving its existing content and layout.Sources: Coding guidelines, Learnings
internal/service/frontend/sse/app_stream_test.go (1)
74-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the other exclusion paths.
This test covers only initial recursive watch-path discovery. Add focused tests for dynamically created
.attachmentsdirectories and Markdown paths handled bysnapshotMarkdownFilesandhandleDocEvent.🤖 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 `@internal/service/frontend/sse/app_stream_test.go` around lines 74 - 85, Expand coverage around recursiveWatchPathsSkipsAttachmentSubtree by adding focused tests for dynamically created .attachments directories and Markdown paths processed by snapshotMarkdownFiles and handleDocEvent. Verify each exclusion path is ignored while preserving expected handling for valid document paths.ui/src/components/docs-live/DocLiveProvider.tsx (1)
41-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing the unused
countsRefmap.
countsReftracks a per-reference count, but no code reads the map.enableddepends only onrefCount, andlookupreadsbyRef. Removing the map reduces the state that must stay consistent.♻️ Proposed simplification
const [refCount, setRefCount] = useState(0); - const countsRef = useRef(new Map<string, number>()); - const registerRef = useCallback((ref: string) => { - const counts = countsRef.current; - counts.set(ref, (counts.get(ref) ?? 0) + 1); + const registerRef = useCallback(() => { setRefCount((c) => c + 1); return () => { - const current = counts.get(ref) ?? 0; - if (current <= 1) { - counts.delete(ref); - } else { - counts.set(ref, current - 1); - } setRefCount((c) => Math.max(0, c - 1)); }; }, []);Keep the
registerRef: (ref: string) => () => voidsignature incontext.tsif you plan to use per-reference counts later.🤖 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 `@ui/src/components/docs-live/DocLiveProvider.tsx` around lines 41 - 54, Remove the unused countsRef map and all per-reference count updates from registerRef in DocLiveProvider. Keep registerRef’s ref parameter and cleanup-returning signature unchanged, and continue updating refCount with the existing increment and nonnegative decrement behavior.ui/src/components/docs-live/__tests__/DocLiveProvider.test.tsx (1)
99-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard both tests against a vacuous pass.
sseCalls.every(...)at Line 107 returnstruefor an empty array. The loop at Line 118 performs no assertion for an empty array. If the provider stops callinguseDAGsListSSE, both tests still pass. Test 1 already guards this at Line 90.♻️ Proposed fix to add non-empty assertions
expect(sseCalls.every((c) => !c.enabled)).toBe(true); + expect(sseCalls.length).toBeGreaterThan(0); }); it('scopes the default doc scope to the default workspace, never all', () => { sseCalls.length = 0; render( <DocLiveProvider workspace={null}> <Probe dagRef="daily-etl" /> </DocLiveProvider> ); + expect(sseCalls.length).toBeGreaterThan(0); for (const call of sseCalls) { expect(call.params).toMatchObject({ workspace: 'default' }); } });🤖 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 `@ui/src/components/docs-live/__tests__/DocLiveProvider.test.tsx` around lines 99 - 121, Add non-empty assertions for sseCalls in both tests: require at least one captured call after rendering DocLiveProvider, before checking enabled flags or workspace parameters. Keep the existing assertions unchanged so the tests still verify the feed is disabled and the default scope is “default,” while preventing vacuous passes if useDAGsListSSE stops being called.ui/src/components/ui/__tests__/doc-markdown-preview.test.tsx (2)
125-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the patched
URLmethods after the test.The test replaces
URL.createObjectURLandURL.revokeObjectURLon the global object and never restores them. Later tests in this file and in the same worker keep the stubs. Usevi.stubGlobalwithvi.unstubAllGlobals(), or restore the original descriptors in anafterEachhook.♻️ Proposed cleanup
it('renders ![[name]] embeds as attachment images', async () => { - Object.defineProperty(URL, 'createObjectURL', { - configurable: true, - value: () => 'blob:attachment-test', - }); - Object.defineProperty(URL, 'revokeObjectURL', { - configurable: true, - value: () => {}, - }); + const createObjectURL = vi + .fn() + .mockReturnValue('blob:attachment-test'); + vi.spyOn(URL, 'createObjectURL').mockImplementation(createObjectURL); + vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => {});Add the restore hook next to the existing
beforeEach:afterEach(() => { vi.restoreAllMocks(); });🤖 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 `@ui/src/components/ui/__tests__/doc-markdown-preview.test.tsx` around lines 125 - 147, Update the attachment-image test around URL.createObjectURL and URL.revokeObjectURL to avoid leaking global patches: use Vitest global stubbing with vi.stubGlobal and add cleanup via vi.unstubAllGlobals(), or capture and restore the original property descriptors in an afterEach hook alongside the existing beforeEach.
161-171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the rendered output, not
container.
expect(container).toBeTruthy()passes for any render result. The test does not prove that the raw name is kept. Assert the anchor title and the absence of a thrown error.♻️ Proposed assertion
- // Must render without throwing; the raw value is kept as the name. - expect(container).toBeTruthy(); + // The raw value is kept as the attachment name. + expect(screen.getByRole('link', { name: 'bad' })).toHaveAttribute( + 'title', + 'Download bad%ZZ.pdf' + ); + expect(container.querySelector('img')).toBeNull();🤖 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 `@ui/src/components/ui/__tests__/doc-markdown-preview.test.tsx` around lines 161 - 171, Update the malformed percent-encoding test for DocMarkdownPreview to assert the rendered attachment link preserves the raw filename in its title, and explicitly verify rendering does not throw. Replace the non-specific container truthiness assertion while retaining coverage for both malformed image and PDF attachment references.ui/src/components/ui/doc-markdown-preview.tsx (1)
288-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one source of truth for the custom fence languages.
The fence language names appear in the
codeoverride and again in thepreoverride. A new block type requires two edits, and a missed edit leaves a stray<pre>wrapper. Extract one map and derive both branches from it.♻️ Proposed refactor
+const FENCE_BLOCKS = { + 'language-mermaid': MermaidBlock, + 'language-dagu-info': DaguInfoBlock, + 'language-dagu-run': DaguRunBlock, +} as const; + code({ className: codeClassName, children }) { - if (codeClassName === 'language-mermaid') { - return <MermaidBlock code={String(children)} />; - } - if (codeClassName === 'language-dagu-info') { - return <DaguInfoBlock source={String(children)} />; - } - if (codeClassName === 'language-dagu-run') { - return <DaguRunBlock source={String(children)} />; - } + if (codeClassName === 'language-mermaid') { + return <MermaidBlock code={String(children)} />; + } + const Block = + codeClassName && codeClassName in FENCE_BLOCKS + ? FENCE_BLOCKS[codeClassName as keyof typeof FENCE_BLOCKS] + : undefined; + if (Block && Block !== MermaidBlock) { + return <Block source={String(children)} />; + } return <code className={codeClassName}>{children}</code>; }, pre({ children }) { const childArray = Array.isArray(children) ? children : [children]; const unwrapped = childArray.some((child) => { if (!isValidElement(child)) return false; - if ( - child.type === MermaidBlock || - child.type === DaguInfoBlock || - child.type === DaguRunBlock - ) { + if ( + Object.values(FENCE_BLOCKS).some((b) => child.type === b) + ) { return true; } const className = (child.props as { className?: string }) .className; - return ( - className === 'language-mermaid' || - className === 'language-dagu-info' || - className === 'language-dagu-run' - ); + return !!className && className in FENCE_BLOCKS; });🤖 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 `@ui/src/components/ui/doc-markdown-preview.tsx` around lines 288 - 321, Extract the custom fence language names into one shared mapping or set near the markdown component, then update both the code override and the pre override to derive their dispatch and unwrapping checks from it. Ensure adding a new custom block requires updating only this centralized definition while preserving the existing MermaidBlock, DaguInfoBlock, and DaguRunBlock behavior.ui/src/pages/docs/components/DocHistoryModal.tsx (1)
69-83: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGuard against out-of-order revision responses.
selectRevisionsetsselectedRevimmediately and applies the response later. If the user selects a second revision before the first response arrives, the slower response can overwrite the newer content. Compare the requestedrevwith the current selection before you apply the result.♻️ Proposed change
const selectedRevRef = useRef<string | null>(null); const selectRevision = async (rev: string) => { setSelectedRev(rev); + selectedRevRef.current = rev; setRevisionContent(null); setLoadError(null); const { data: revData, error } = await client.GET('/docs/doc/revision', { params: { query: { remoteNode, path: docPath, rev, ...workspaceQuery }, }, }); + if (selectedRevRef.current !== rev) return; if (error || !revData) {🤖 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 `@ui/src/pages/docs/components/DocHistoryModal.tsx` around lines 69 - 83, Update selectRevision to ignore stale asynchronous responses: after client.GET resolves, verify the requested rev still matches the current selected revision before applying either the load error or revision content. Preserve the existing behavior for the latest selection while preventing an earlier response from overwriting it.ui/src/pages/docs/components/CreateDocModal.tsx (1)
149-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a smaller select height.
The coding guidelines require select boxes with a height of
h-7or smaller.SelectTriggerusesh-8here.♻️ Proposed change
- <SelectTrigger id="doc-template" className="col-span-3 h-8"> + <SelectTrigger id="doc-template" size="sm" className="col-span-3 h-7">🤖 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 `@ui/src/pages/docs/components/CreateDocModal.tsx` at line 149, Update the SelectTrigger for doc-template in CreateDocModal to use h-7 or smaller instead of h-8, preserving the existing styling and behavior.Source: Coding guidelines
ui/src/pages/docs/components/DocTreeSidebar.tsx (1)
764-775: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the clear action part of the menu.
The "Clear filter" control is a plain
buttoninsideDropdownMenuContent. Radix keyboard navigation moves between menu items only, so this control is not reachable with the arrow keys. UseDropdownMenuIteminstead.♻️ Proposed change
- <button - type="button" - className="w-full text-left text-xs px-2 py-1 text-muted-foreground hover:bg-accent rounded-sm" - onClick={() => setSelectedTags([])} - > - Clear filter - </button> + <DropdownMenuItem + className="text-xs text-muted-foreground" + onSelect={() => setSelectedTags([])} + > + Clear filter + </DropdownMenuItem>Add
DropdownMenuItemto the import at Line 9-18.🤖 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 `@ui/src/pages/docs/components/DocTreeSidebar.tsx` around lines 764 - 775, Replace the plain “Clear filter” button in the selected-tags section of DocTreeSidebar with DropdownMenuItem, preserving its onClick behavior and styling as appropriate; also add DropdownMenuItem to the existing Radix menu imports.ui/src/pages/docs/components/DocBacklinksPanel.tsx (1)
41-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExpose the collapse state to assistive technology.
The toggle button changes the visibility of the backlink list. Add
aria-expandedso screen readers report the state.♻️ Proposed change
<button type="button" onClick={() => setCollapsed((c) => !c)} + aria-expanded={!collapsed} className="w-full flex items-center gap-1 px-3 py-1.5 text-xs font-medium uppercase tracking-wide text-muted-foreground hover:text-foreground" >🤖 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 `@ui/src/pages/docs/components/DocBacklinksPanel.tsx` around lines 41 - 45, Add an aria-expanded attribute to the collapse toggle button in DocBacklinksPanel, binding it to the current collapsed state with the correct expanded-value inversion. Keep the existing setCollapsed toggle behavior unchanged.ui/src/pages/docs/lib/doc-templates.ts (1)
11-12: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider delimiting the placeholder token.
DOC_TEMPLATE_DAG_NAMEis the bare tokenDAG_NAME. Consumers replace every occurrence withsplit(...).join(...)(seeui/src/features/dags/components/dag-details/DAGDocsTab.tsxline 153). The replacement also hits substrings inside larger identifiers, for exampleMY_DAG_NAME_SUFFIX, and any prose that mentionsDAG_NAME. A delimited token avoids this.♻️ Proposed change
-export const DOC_TEMPLATE_DAG_NAME = 'DAG_NAME'; +export const DOC_TEMPLATE_DAG_NAME = '{{DAG_NAME}}';Built-in template bodies already interpolate the constant, so they need no edit. Existing user templates that use the old token require a migration note.
🤖 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 `@ui/src/pages/docs/lib/doc-templates.ts` around lines 11 - 12, Change the value of DOC_TEMPLATE_DAG_NAME to a clearly delimited placeholder token so replacements only target the intended marker, and add a migration note for existing user templates that still use the bare DAG_NAME token. Keep built-in templates relying on DOC_TEMPLATE_DAG_NAME unchanged.internal/persis/file/doc/store_test.go (1)
592-623: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a non-matching document to the ranking fixtures.
Every fixture in both tests contains the query term. The tests therefore cannot detect whether
Searchexcludes documents with no match. That gap hides the filtering defect flagged ininternal/persis/file/doc/search.goLines 50-78.Add a document that does not contain the query and assert it is absent from the results.
💚 Proposed addition
require.NoError(t, store.Create(ctx, "m-single", "one etl mention")) + // A document with no match must not appear in the results. + require.NoError(t, store.Create(ctx, "no-match", "completely unrelated body")) results, err := store.Search(ctx, "etl") require.NoError(t, err) require.Len(t, results, 3)🤖 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 `@internal/persis/file/doc/store_test.go` around lines 592 - 623, Add a fixture document without the query term to both TestSearchRanking and TestSearchRankingTiebreakByID, then assert the returned results contain only the matching documents and exclude the non-matching document.internal/persis/file/doc/store.go (1)
112-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard
WithDataDiragainst an empty directory.
filepath.Clean("")returns".". An empty argument therefore enables revisions and writesrevisions.jsonplus blobs into the process working directory.NewDocStorecurrently checkscfg.Paths.DataDir != ""before calling this option, so production is safe today. Add the guard inside the option so a future caller cannot break it.♻️ Proposed guard
func WithDataDir(dir string) Option { return func(s *Store) { + if dir == "" { + return + } s.dataDir = filepath.Clean(dir) } }🤖 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 `@internal/persis/file/doc/store.go` around lines 112 - 116, Update WithDataDir to ignore empty dir values before calling filepath.Clean or assigning Store.dataDir; preserve the existing behavior for non-empty directories.internal/persis/file/doc/revision.go (1)
58-96: 🚀 Performance & Scalability | 🔵 TrivialConsider the whole-file manifest rewrite as the document count grows.
revisions.jsonholds every document's revision list. Each snapshot, delete, and rename loads and rewrites the entire file whilemutationMuis held. Cost grows linearly with the number of documents that have history, up to 20 entries each. This is acceptable for a wiki-scale workload. If document counts grow large, move to a per-document manifest under the revisions directory.🤖 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 `@internal/persis/file/doc/revision.go` around lines 58 - 96, Keep the current whole-file revisions manifest implementation for the wiki-scale workload, but document the scalability boundary around loadRevisionsManifest and saveRevisionsManifest: note that each snapshot, delete, and rename rewrites all document revision lists while mutationMu is held, and identify a per-document manifest under the revisions directory as the future approach if document counts become large.
🤖 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 `@internal/core/docs/link.go`:
- Around line 37-45: Update the line-processing logic around the
inlineCodeRegexp and wiki-link matching to preserve inline-code delimiter state
across lines, ensuring wiki links inside multi-line code spans are ignored while
same-line spans retain current behavior. Add a regression test covering a
multi-line inline-code span containing a [[target]] link and verify no backlink
is created.
In `@internal/gitsync/git.go`:
- Around line 586-597: Update the filesystem traversal around the directory
handling and relPath computation to skip any entry whose d.Type().IsRegular() is
false, including symlinks and other non-regular entries. Perform this check
before filepath.Rel and appending to files, while preserving the existing .git
directory SkipDir behavior.
In `@internal/gitsync/service.go`:
- Around line 1698-1704: Update the remote metadata branch around
GetFileSizeAtCommit to call s.gitClient.Open() before reading the remote file
size, and handle the open error explicitly instead of silently continuing.
Preserve diff.RemoteCommit assignment while ensuring repository-open or
size-lookup failures are returned or represented according to the surrounding
service error-handling contract.
- Around line 138-147: Change SyncItemDiff’s binary size representation to
preserve unavailable values, using pointer sizes or explicit availability
fields, and assign them only after successful local stat or remote lookup.
Update the mapper in the sync API flow to serialize unavailable sizes
distinctly, then adjust affected tests to verify zero-byte sizes remain valid
while missing sizes trigger the UI fallback.
In `@internal/persis/file/doc/attachment.go`:
- Around line 47-74: Update PutAttachment to call io.ReadAll(content) before
acquiring mutationMu, returning the existing wrapped read error immediately if
buffering fails. Then acquire mutationMu and keep the document existence check,
directory creation, and atomic file write inside the locked filesystem-work
section.
In `@internal/service/frontend/api/v1/docs.go`:
- Around line 340-346: Update the oversized attachment branch in
UploadDocAttachment to use api.ErrorCodePayloadTooLarge instead of
api.ErrorCodeBadRequest, while preserving the existing HTTP 413 status and
message.
In `@specs/034-doc-format.md`:
- Around line 1-126: The spec 034 entry is marked Implemented without tracked
conformance coverage. Add conformance/spec034_doc_format coverage for the
document-format behaviors defined in specs/034-doc-format.md, then update
specs/034-doc-format.md and specs/README.md to reflect the actual available
conformance results; at specs/034-doc-format.md lines 1-126 and specs/README.md
lines 38-39, make the status and listing consistent, with no direct change
required to the README beyond aligning it with the coverage status.
In `@ui/src/components/docs-live/DagStatusChip.tsx`:
- Around line 29-49: Update the loading and not-found branches in DagStatusChip
to render the computed text directly, preserving an explicitly empty label as an
empty chip. Keep the dagRef fallback only for the undefined-label case handled
when text is initialized, and leave the found branch unchanged.
In `@ui/src/components/docs-live/DocLiveProvider.tsx`:
- Around line 79-88: Update the byRef useMemo map construction in
DocLiveProvider to enforce deterministic name-first resolution: first index
every item.dag.name, then add item.fileName entries only when those keys are not
already mapped. Preserve the existing first-item-wins behavior for duplicate
logical names and file names.
In `@ui/src/features/dags/components/dag-details/DAGDetailsContent.tsx`:
- Around line 299-305: Remove the non-modal Docs LinkTab from the DAG details
tab list, including its surrounding conditional if it is only used by this tab.
Leave the modal docs behavior and other tabs unchanged, so DAG details no longer
links to the generic /docs route.
In `@ui/src/features/dags/components/dag-details/DAGDocsTab.tsx`:
- Around line 144-172: Update handleCreate to catch rejections from client.POST
and set createError to the request error message or a suitable fallback, while
retaining the existing success flow and finally block that clears loading state.
- Around line 38-44: Update docLink and the navigate call in DAGDocsTab to
encode the document ID using the same per-segment encodeURIComponent rule as
encodeDocPathForURL, replacing encodeURI while preserving the existing workspace
query handling so both document-link surfaces produce identical URLs.
In `@ui/src/lib/remark-wikilink.ts`:
- Around line 24-40: Remove the decodeURIComponent try/catch and related
decoding comments from parseWikilinkHref, preserving href slicing and hash-based
target/anchor parsing so percent sequences remain literal.
In `@ui/src/pages/docs/components/DocEditor.tsx`:
- Around line 320-322: Update uploadFiles to await each uploadAttachment call
sequentially rather than starting all uploads concurrently, preserving
completion order and ensuring currentValueRef.current includes previously
appended Markdown links before the next setCurrentValue update; keep the
existing selection and no-selection handling unchanged.
In `@ui/src/pages/docs/components/DocHistoryModal.tsx`:
- Around line 109-112: Update the empty-content branch in DocHistoryModal to use
the useQuery loading flag: display a loading message while the revisions request
is in flight, and show “No stored revisions yet” only after loading completes
with no revisions. Preserve the existing revisions list rendering.
---
Nitpick comments:
In `@internal/persis/file/doc/revision.go`:
- Around line 58-96: Keep the current whole-file revisions manifest
implementation for the wiki-scale workload, but document the scalability
boundary around loadRevisionsManifest and saveRevisionsManifest: note that each
snapshot, delete, and rename rewrites all document revision lists while
mutationMu is held, and identify a per-document manifest under the revisions
directory as the future approach if document counts become large.
In `@internal/persis/file/doc/store_test.go`:
- Around line 592-623: Add a fixture document without the query term to both
TestSearchRanking and TestSearchRankingTiebreakByID, then assert the returned
results contain only the matching documents and exclude the non-matching
document.
In `@internal/persis/file/doc/store.go`:
- Around line 112-116: Update WithDataDir to ignore empty dir values before
calling filepath.Clean or assigning Store.dataDir; preserve the existing
behavior for non-empty directories.
In `@internal/service/frontend/sse/app_stream_test.go`:
- Around line 74-85: Expand coverage around
recursiveWatchPathsSkipsAttachmentSubtree by adding focused tests for
dynamically created .attachments directories and Markdown paths processed by
snapshotMarkdownFiles and handleDocEvent. Verify each exclusion path is ignored
while preserving expected handling for valid document paths.
In `@ui/src/components/docs-live/__tests__/DocLiveProvider.test.tsx`:
- Around line 99-121: Add non-empty assertions for sseCalls in both tests:
require at least one captured call after rendering DocLiveProvider, before
checking enabled flags or workspace parameters. Keep the existing assertions
unchanged so the tests still verify the feed is disabled and the default scope
is “default,” while preventing vacuous passes if useDAGsListSSE stops being
called.
In `@ui/src/components/docs-live/DocLiveProvider.tsx`:
- Around line 41-54: Remove the unused countsRef map and all per-reference count
updates from registerRef in DocLiveProvider. Keep registerRef’s ref parameter
and cleanup-returning signature unchanged, and continue updating refCount with
the existing increment and nonnegative decrement behavior.
In `@ui/src/components/ui/__tests__/doc-markdown-preview.test.tsx`:
- Around line 125-147: Update the attachment-image test around
URL.createObjectURL and URL.revokeObjectURL to avoid leaking global patches: use
Vitest global stubbing with vi.stubGlobal and add cleanup via
vi.unstubAllGlobals(), or capture and restore the original property descriptors
in an afterEach hook alongside the existing beforeEach.
- Around line 161-171: Update the malformed percent-encoding test for
DocMarkdownPreview to assert the rendered attachment link preserves the raw
filename in its title, and explicitly verify rendering does not throw. Replace
the non-specific container truthiness assertion while retaining coverage for
both malformed image and PDF attachment references.
In `@ui/src/components/ui/doc-markdown-preview.tsx`:
- Around line 288-321: Extract the custom fence language names into one shared
mapping or set near the markdown component, then update both the code override
and the pre override to derive their dispatch and unwrapping checks from it.
Ensure adding a new custom block requires updating only this centralized
definition while preserving the existing MermaidBlock, DaguInfoBlock, and
DaguRunBlock behavior.
In `@ui/src/pages/docs/components/CreateDocModal.tsx`:
- Line 149: Update the SelectTrigger for doc-template in CreateDocModal to use
h-7 or smaller instead of h-8, preserving the existing styling and behavior.
In `@ui/src/pages/docs/components/DocBacklinksPanel.tsx`:
- Around line 41-45: Add an aria-expanded attribute to the collapse toggle
button in DocBacklinksPanel, binding it to the current collapsed state with the
correct expanded-value inversion. Keep the existing setCollapsed toggle behavior
unchanged.
In `@ui/src/pages/docs/components/DocHistoryModal.tsx`:
- Around line 69-83: Update selectRevision to ignore stale asynchronous
responses: after client.GET resolves, verify the requested rev still matches the
current selected revision before applying either the load error or revision
content. Preserve the existing behavior for the latest selection while
preventing an earlier response from overwriting it.
In `@ui/src/pages/docs/components/DocTreeSidebar.tsx`:
- Around line 764-775: Replace the plain “Clear filter” button in the
selected-tags section of DocTreeSidebar with DropdownMenuItem, preserving its
onClick behavior and styling as appropriate; also add DropdownMenuItem to the
existing Radix menu imports.
In `@ui/src/pages/docs/lib/doc-templates.ts`:
- Around line 11-12: Change the value of DOC_TEMPLATE_DAG_NAME to a clearly
delimited placeholder token so replacements only target the intended marker, and
add a migration note for existing user templates that still use the bare
DAG_NAME token. Keep built-in templates relying on DOC_TEMPLATE_DAG_NAME
unchanged.
In `@ui/src/pages/git-sync/DiffModal.tsx`:
- Around line 108-137: Update the binary attachment information block in the
binary rendering branch to use compact padding such as p-3 instead of p-6, and
add the semantic bg-muted/30 background class while preserving its existing
content and layout.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 64eab593-0ec7-4c0f-bd5a-cbe52862b6a3
⛔ Files ignored due to path filters (1)
ui/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (66)
api/v1/api.gen.goapi/v1/api.yamlinternal/core/docs/doc.gointernal/core/docs/link.gointernal/core/docs/link_test.gointernal/gitsync/git.gointernal/gitsync/pull_external_test.gointernal/gitsync/service.gointernal/gitsync/service_test.gointernal/gitsync/state.gointernal/persis/file/doc/attachment.gointernal/persis/file/doc/attachment_test.gointernal/persis/file/doc/backlinks.gointernal/persis/file/doc/backlinks_test.gointernal/persis/file/doc/revision.gointernal/persis/file/doc/revision_test.gointernal/persis/file/doc/search.gointernal/persis/file/doc/store.gointernal/persis/file/doc/store_test.gointernal/persis/file/doc/tree.gointernal/persis/file/service_stores.gointernal/service/frontend/api/v1/docs.gointernal/service/frontend/api/v1/docs_response.gointernal/service/frontend/api/v1/docs_test.gointernal/service/frontend/api/v1/search.gointernal/service/frontend/api/v1/sync.gointernal/service/frontend/api/v1/sync_test.gointernal/service/frontend/sse/app_stream.gointernal/service/frontend/sse/app_stream_test.gointernal/service/mcp/docs_tool.gospecs/034-doc-format.mdspecs/README.mdui/package.jsonui/src/api/v1/schema.tsui/src/components/docs-live/DagStatusChip.tsxui/src/components/docs-live/DaguInfoBlock.tsxui/src/components/docs-live/DaguRunBlock.tsxui/src/components/docs-live/DocLiveProvider.tsxui/src/components/docs-live/__tests__/DaguRunBlock.test.tsxui/src/components/docs-live/__tests__/DocLiveProvider.test.tsxui/src/components/docs-live/context.tsui/src/components/editors/MarkdownEditor.tsxui/src/components/ui/__tests__/doc-markdown-preview.test.tsxui/src/components/ui/doc-markdown-preview.cssui/src/components/ui/doc-markdown-preview.tsxui/src/features/dags/components/dag-details/DAGDetailsContent.tsxui/src/features/dags/components/dag-details/DAGDocsTab.tsxui/src/features/dags/components/dag-details/__tests__/DAGDetailsContent.test.tsxui/src/hooks/useDocAttachmentUrl.tsui/src/lib/remark-wikilink.tsui/src/pages/docs/components/CreateDocModal.tsxui/src/pages/docs/components/DocBacklinksPanel.tsxui/src/pages/docs/components/DocEditor.tsxui/src/pages/docs/components/DocHistoryModal.tsxui/src/pages/docs/components/DocTreeSidebar.tsxui/src/pages/docs/components/__tests__/CreateDocModal.test.tsxui/src/pages/docs/components/__tests__/DocEditor.test.tsxui/src/pages/docs/hooks/useDocTemplates.tsui/src/pages/docs/index.tsxui/src/pages/docs/lib/doc-attachments.tsui/src/pages/docs/lib/doc-templates.tsui/src/pages/git-sync/DiffModal.tsxui/src/pages/git-sync/__tests__/DiffModal.test.tsxui/src/pages/git-sync/__tests__/sync-kind.test.tsui/src/pages/git-sync/index.tsxui/src/pages/git-sync/sync-kind.ts
Member
Author
|
Addressed the review summary nitpicks in I left two suggestions unchanged after checking them: Local validation is green for the affected Go packages and conformance test, all frontend tests, TypeScript checking, and ESLint on changed frontend files. |
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/src/features/dags/components/dag-details/DAGDocsTab.tsx`:
- Around line 200-202: Update the wikilink example in the DAGDocsTab render so
dagName is interpolated as JSX text rather than displayed literally. Preserve
the surrounding explanatory text and existing validSegment-based path rendering.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cb3657b-7c2d-43af-914d-f5975e66de95
📒 Files selected for processing (38)
conformance/spec034_doc_format/doc_format_test.gointernal/core/docs/link.gointernal/core/docs/link_test.gointernal/gitsync/git.gointernal/gitsync/git_test.gointernal/gitsync/pull_external_test.gointernal/gitsync/service.gointernal/persis/file/doc/attachment.gointernal/persis/file/doc/revision_test.gointernal/persis/file/doc/store.gointernal/persis/file/doc/store_test.gointernal/service/frontend/api/v1/docs.gointernal/service/frontend/api/v1/docs_test.gointernal/service/frontend/api/v1/sync.gointernal/service/frontend/api/v1/sync_test.gointernal/service/frontend/sse/app_stream_test.goui/src/components/docs-live/DagStatusChip.tsxui/src/components/docs-live/DocLiveProvider.tsxui/src/components/docs-live/__tests__/DaguRunBlock.test.tsxui/src/components/docs-live/__tests__/DocLiveProvider.test.tsxui/src/components/ui/__tests__/doc-markdown-preview.test.tsxui/src/components/ui/doc-markdown-preview.tsxui/src/features/dags/components/__tests__/DAGStatus.test.tsxui/src/features/dags/components/dag-details/DAGDetailsContent.tsxui/src/features/dags/components/dag-details/DAGDocsTab.tsxui/src/features/dags/components/dag-details/__tests__/DAGDetailsContent.test.tsxui/src/features/search/components/SearchResult.tsxui/src/lib/remark-wikilink.tsui/src/pages/docs/components/CreateDocModal.tsxui/src/pages/docs/components/DocBacklinksPanel.tsxui/src/pages/docs/components/DocEditor.tsxui/src/pages/docs/components/DocHistoryModal.tsxui/src/pages/docs/components/DocTreeSidebar.tsxui/src/pages/docs/components/__tests__/DocEditor.test.tsxui/src/pages/docs/index.tsxui/src/pages/docs/lib/doc-path.tsui/src/pages/docs/lib/doc-templates.tsui/src/pages/git-sync/DiffModal.tsx
🚧 Files skipped from review as they are similar to previous changes (22)
- internal/core/docs/link_test.go
- ui/src/components/docs-live/tests/DaguRunBlock.test.tsx
- internal/gitsync/git.go
- internal/core/docs/link.go
- ui/src/components/ui/tests/doc-markdown-preview.test.tsx
- internal/gitsync/pull_external_test.go
- ui/src/features/dags/components/dag-details/tests/DAGDetailsContent.test.tsx
- internal/persis/file/doc/attachment.go
- internal/service/frontend/api/v1/sync.go
- internal/service/frontend/api/v1/docs_test.go
- internal/persis/file/doc/store.go
- internal/gitsync/service.go
- ui/src/pages/docs/components/DocBacklinksPanel.tsx
- ui/src/pages/docs/components/DocTreeSidebar.tsx
- ui/src/components/docs-live/tests/DocLiveProvider.test.tsx
- ui/src/pages/docs/components/CreateDocModal.tsx
- ui/src/pages/git-sync/DiffModal.tsx
- ui/src/pages/docs/lib/doc-templates.ts
- ui/src/pages/docs/components/DocEditor.tsx
- ui/src/pages/docs/index.tsx
- ui/src/pages/docs/components/DocHistoryModal.tsx
- ui/src/lib/remark-wikilink.ts
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/src/features/dags/components/dag-details/__tests__/DAGDocsTab.test.tsx (1)
4-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReorder imports by dependency group.
Place the external imports before
@/contexts/AppBarContextand../DAGDocsTab. Add a blank line between the external and internal groups.As per coding guidelines, keep imports organized with external packages first, then internal modules.
Proposed import order
-import { AppBarContext } from '`@/contexts/AppBarContext`'; import { render, screen } from '`@testing-library/react`'; import React from 'react'; import { MemoryRouter } from 'react-router-dom'; import { describe, expect, it, vi } from 'vitest'; + +import { AppBarContext } from '`@/contexts/AppBarContext`'; import DAGDocsTab from '../DAGDocsTab';🤖 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 `@ui/src/features/dags/components/dag-details/__tests__/DAGDocsTab.test.tsx` around lines 4 - 9, Reorder imports in DAGDocsTab.test.tsx so external packages come first, followed by a blank line, then the internal AppBarContext and DAGDocsTab imports.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@ui/src/features/dags/components/dag-details/__tests__/DAGDocsTab.test.tsx`:
- Around line 4-9: Reorder imports in DAGDocsTab.test.tsx so external packages
come first, followed by a blank line, then the internal AppBarContext and
DAGDocsTab imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ecafc329-146e-46f8-895e-b2692c7cab04
📒 Files selected for processing (2)
ui/src/features/dags/components/dag-details/DAGDocsTab.tsxui/src/features/dags/components/dag-details/__tests__/DAGDocsTab.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/src/features/dags/components/dag-details/DAGDocsTab.tsx
Member
Author
|
Addressed the final CodeRabbit import-grouping nitpick in |
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Keep editor upload callbacks current across document path changes and clean up DOM listeners on disposal. Centralize API-compatible attachment naming and reuse the shared workspace query helper.
yohamta0
force-pushed
the
feat/docs-wiki
branch
from
August 9, 2026 02:15
315838a to
8250e76
Compare
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.
Summary
Why
Documents were isolated Markdown files without enough navigation, history, operational context, or asset support for runbooks. This makes them useful as a connected workspace-aware wiki while keeping the file-backed storage model and Git Sync workflow.
Impact
Authors can build linked operational documentation, embed current DAG information, run approved workflows from runbooks, restore earlier document versions, and synchronize attachments with the rest of the repository. Workspace permissions continue to scope document and DAG operations.
Testing
go test ./internal/core/docs ./internal/persis/file/doc ./internal/gitsync ./internal/service/frontend/api/v1pnpm test -- --runInBand(fromui)pnpm typecheck(fromui)pnpm build(fromui)Summary by cubic
Turned Documents into an operations wiki with tags, wikilinks/backlinks, templates, live DAG embeds and run actions, local revision history, and binary attachments synchronized via Git with binary-aware diffs.
New Features
[[target]]with anchors/labels and backlinks (API/docs/backlinks+ UI); Obsidian-style embeds![[image.png]].docs/.attachments/{docID}/{name}with paste/drop upload, safe names, image embeds; upload/download APIs; Git Syncdoc-assetkind with size-based diffs; SSE/watchers skip.attachments._templates/per workspace), and a DAG “Docs” tab to link/create runbooks./search/docand flat/docs, revisions endpoints, binary-aware Git Sync diff; docs sidebar tag filters, backlinks panel, wikilink rendering, and attachment handling in Git Sync.Bug Fixes
Written for commit 0ba6d87. Summary will update on new commits.
Summary by CodeRabbit