Repository navigation
refactor: Remove docs feature - #2331
Merged
Merged
Conversation
📝 WalkthroughWait, I have I realize WalkthroughRemoves the Docs/Runbooks feature end-to-end: deletes ChangesDocs/Runbooks Feature Removal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
internal/tools/llmsgen/generator.go (1)
42-47: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRegenerate
llms.txtto match the new header
The committedllms.txtstill has the old docs URL and is missingIt summarizes repository-local workflow references.Runmake llmsso the checked-in artifact stays in sync.🤖 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/tools/llmsgen/generator.go` around lines 42 - 47, The llms.txt generator header has changed, but the committed llms.txt is still stale and missing the new sentence, so regenerate the artifact to keep it in sync. Update the checked-in llms.txt by running the llms generation flow driven by generator.go so the new header text from the generator is reflected exactly in the repo.internal/service/frontend/api/v1/sync.go (1)
701-729: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDon’t coerce stale sync kinds into DAG items.
With
docremoved fromSyncItemKind, stale persisted doc states now fall through asdagand get a.yamlpath. Filter unsupported kinds instead of misrepresenting them in/sync/status.Proposed fix
-func toAPISyncItemKind(itemID string, kind gitsync.DAGKind) api.SyncItemKind { +func toAPISyncItemKind(itemID string, kind gitsync.DAGKind) (api.SyncItemKind, bool) { kind = resolveKind(itemID, kind) switch kind { case gitsync.DAGKindMemory: - return api.SyncItemKindMemory + return api.SyncItemKindMemory, true case gitsync.DAGKindSkill: - return api.SyncItemKindSkill + return api.SyncItemKindSkill, true case gitsync.DAGKindSoul: - return api.SyncItemKindSoul + return api.SyncItemKindSoul, true case gitsync.DAGKindConfig: - return api.SyncItemKindConfig + return api.SyncItemKindConfig, true case gitsync.DAGKindDAG: - return api.SyncItemKindDag + return api.SyncItemKindDag, true default: - return api.SyncItemKindDag + return "", false } } -func syncItemFilePath(itemID string, kind gitsync.DAGKind) string { +func syncItemFilePath(itemID string, kind gitsync.DAGKind) (string, bool) { kind = resolveKind(itemID, kind) ext := ".yaml" switch kind { case gitsync.DAGKindMemory, gitsync.DAGKindSkill, gitsync.DAGKindSoul: ext = ".md" case gitsync.DAGKindDAG, gitsync.DAGKindConfig: // default .yaml extension + default: + return "", false } - return itemID + ext + return itemID + ext, true }- filePath := syncItemFilePath(itemID, state.Kind) + filePath, ok := syncItemFilePath(itemID, state.Kind) + if !ok { + continue + } + itemKind, ok := toAPISyncItemKind(itemID, state.Kind) + if !ok { + continue + } item := api.SyncItem{ @@ - Kind: toAPISyncItemKind(itemID, state.Kind), + Kind: itemKind,Also applies to: 742-748
🤖 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/api/v1/sync.go` around lines 701 - 729, The sync kind mapping in toAPISyncItemKind and syncItemFilePath is coercing unsupported stale kinds into DAG, which misrepresents removed doc states in /sync/status. Update resolveKind handling so unsupported kinds are filtered out or treated as invalid instead of falling through to gitsync.DAGKindDAG, and make both toAPISyncItemKind and syncItemFilePath return a non-DAG/empty result for stale kinds rather than assigning a .yaml path. Use the existing resolveKind, toAPISyncItemKind, and syncItemFilePath symbols to keep the behavior consistent for supported gitsync.DAGKind values only.ui/src/pages/git-sync/RowActionMenu.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the GPL v3 source header.
This TSX source file starts directly with imports, so it is missing the required license header. As per coding guidelines, “Add GPL v3 license headers to source files.”
Proposed fix
+// Copyright (C) 2026 Yota Hamada +// SPDX-License-Identifier: GPL-3.0-or-later + import { SyncStatus } from '`@/api/v1/schema`';🤖 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/RowActionMenu.tsx` at line 1, The RowActionMenu TSX file is missing the required GPL v3 source header at the top of the module. Update the file so the license header appears before the existing imports, following the same GPL v3 source header format used in other source files. Keep the rest of the RowActionMenu component and its SyncStatus import unchanged.Source: Coding guidelines
ui/src/pages/git-sync/index.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the GPL v3 source header.
This TSX source file starts directly with imports, so it is missing the required license header. As per coding guidelines, “Add GPL v3 license headers to source files.”
Proposed fix
+// Copyright (C) 2026 Yota Hamada +// SPDX-License-Identifier: GPL-3.0-or-later + import {🤖 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/index.tsx` at line 1, The TSX module is missing the required GPL v3 source header at the top of the file. Add the standard GPL v3 license comment block before the existing import section in this page component so the file begins with the header and then the current imports and page logic.Source: Coding guidelines
internal/gitsync/service.go (1)
257-267: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFilter unsupported
.mdentries out ofrepoFileSetas well.Right now legacy
docs/*.mdfiles are skipped for syncing, but they were already added torepoFileSeta few lines earlier. That meansreconcileAfterPullstill sees them as “present in remote”, so pre-removal docs state can linger indefinitely after an upgrade instead of being forgotten.Suggested fix
- // Build set of DAG IDs present in remote repo for reconcileAfterPull - repoFileSet := make(map[string]struct{}, len(files)) - for _, file := range files { - repoFileSet[s.filePathToDAGID(file)] = struct{}{} - } + // Build set of supported DAG IDs present in remote repo for reconcileAfterPull + repoFileSet := make(map[string]struct{}, len(files)) for _, file := range files { dagID := s.filePathToDAGID(file) // Only allow .md files from memory/, skills/, or souls/ directories. if filepath.Ext(file) == ".md" && !isMemoryFile(dagID) && !isSkillFile(dagID) && !isSoulFile(dagID) { continue } + repoFileSet[dagID] = struct{}{}🤖 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/gitsync/service.go` around lines 257 - 267, The unsupported `.md` entries are still being added to repoFileSet before the sync filter runs, so legacy docs can remain marked as present. Update the file handling in service.go around the repoFileSet build and the subsequent file loop so the same .md allowlist used in the loop (memory/, skills/, souls/) is applied when populating repoFileSet, and keep reconcileAfterPull from seeing skipped docs as remote-present.
🤖 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/agent/system_prompt.txt`:
- Around line 10-13: The system prompt text incorrectly refers to nonexistent
“memory tool” and “skill tool” actions, which can mislead the agent. Update the
wording in the prompt to match the available mechanisms, using the actual memory
and skill interfaces exposed elsewhere in the agent flow (for example the
file-backed memory read/patch path and skill discovery via search_skills and
delegate.skills). Keep the guidance about what belongs in memory vs skills, but
replace the unsupported tool names with the real symbols so the prompt stays
accurate.
In `@ui/src/features/search/components/SearchResult.tsx`:
- Around line 189-207: The SearchResult loadMore callback currently only reads
response.error, so a thrown client/network failure from client.GET can escape
instead of producing a LoadMoreResponse error. Update the loadMore async
function in SearchResult to wrap the request in try/catch, keep the existing
response.data mapping on success, and return a LoadMoreResponse with an error
string in the catch path so failures are always surfaced consistently.
---
Outside diff comments:
In `@internal/gitsync/service.go`:
- Around line 257-267: The unsupported `.md` entries are still being added to
repoFileSet before the sync filter runs, so legacy docs can remain marked as
present. Update the file handling in service.go around the repoFileSet build and
the subsequent file loop so the same .md allowlist used in the loop (memory/,
skills/, souls/) is applied when populating repoFileSet, and keep
reconcileAfterPull from seeing skipped docs as remote-present.
In `@internal/service/frontend/api/v1/sync.go`:
- Around line 701-729: The sync kind mapping in toAPISyncItemKind and
syncItemFilePath is coercing unsupported stale kinds into DAG, which
misrepresents removed doc states in /sync/status. Update resolveKind handling so
unsupported kinds are filtered out or treated as invalid instead of falling
through to gitsync.DAGKindDAG, and make both toAPISyncItemKind and
syncItemFilePath return a non-DAG/empty result for stale kinds rather than
assigning a .yaml path. Use the existing resolveKind, toAPISyncItemKind, and
syncItemFilePath symbols to keep the behavior consistent for supported
gitsync.DAGKind values only.
In `@internal/tools/llmsgen/generator.go`:
- Around line 42-47: The llms.txt generator header has changed, but the
committed llms.txt is still stale and missing the new sentence, so regenerate
the artifact to keep it in sync. Update the checked-in llms.txt by running the
llms generation flow driven by generator.go so the new header text from the
generator is reflected exactly in the repo.
In `@ui/src/pages/git-sync/index.tsx`:
- Line 1: The TSX module is missing the required GPL v3 source header at the top
of the file. Add the standard GPL v3 license comment block before the existing
import section in this page component so the file begins with the header and
then the current imports and page logic.
In `@ui/src/pages/git-sync/RowActionMenu.tsx`:
- Line 1: The RowActionMenu TSX file is missing the required GPL v3 source
header at the top of the module. Update the file so the license header appears
before the existing imports, following the same GPL v3 source header format used
in other source files. Keep the rest of the RowActionMenu component and its
SyncStatus import unchanged.
🪄 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: e240ceb2-09b8-4086-a91e-f30b0355a324
⛔ Files ignored due to path filters (2)
assets/images/readme-documents-dark.pngis excluded by!**/*.pngui/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (109)
README.mdapi/v1/api.gen.goapi/v1/api.yamlinternal/agent/api.gointernal/agent/doc.gointernal/agent/navigate.gointernal/agent/runbook_manage.gointernal/agent/runbook_manage_test.gointernal/agent/session.gointernal/agent/session_search_test.gointernal/agent/system_prompt.txtinternal/agent/system_prompt_test.gointernal/agent/tool_registry.gointernal/agent/tool_registry_test.gointernal/agent/types.gointernal/cmd/agent.gointernal/cmd/config.gointernal/cmd/config_test.gointernal/cmd/process/frontend_store_factories.gointernal/cmn/config/config.gointernal/cmn/config/definition.gointernal/cmn/config/loader.gointernal/cmn/config/loader_test.gointernal/cmn/value/template.gointernal/core/exec/context.gointernal/core/exec/context_env.gointernal/core/exec/context_env_test.gointernal/core/exec/context_test.gointernal/core/exec/env.gointernal/core/spec/builder.gointernal/gitsync/service.gointernal/gitsync/service_test.gointernal/gitsync/state.gointernal/persis/file/doc/birthtime_darwin.gointernal/persis/file/doc/birthtime_linux.gointernal/persis/file/doc/birthtime_other.gointernal/persis/file/doc/store.gointernal/persis/file/doc/store_test.gointernal/persis/file/service_stores.gointernal/runtime/builtin/agentstep/executor.gointernal/runtime/builtin/harness/harness.gointernal/runtime/builtin/harness/root_container_external_test.gointernal/runtime/builtin_context_test.gointernal/runtime/env_test.gointernal/runtime/eval.gointernal/service/frontend/api/v1/api.gointernal/service/frontend/api/v1/docs.gointernal/service/frontend/api/v1/docs_test.gointernal/service/frontend/api/v1/search.gointernal/service/frontend/api/v1/search_test.gointernal/service/frontend/api/v1/sync.gointernal/service/frontend/persistence.gointernal/service/frontend/server.gointernal/service/frontend/server_test.gointernal/service/frontend/sse/app_stream.gointernal/service/frontend/sse/multiplex_test.gointernal/service/frontend/sse/topic_parse.gointernal/service/frontend/sse/types.gointernal/tools/llmsgen/generator.goui/package.jsonui/src/App.tsxui/src/__tests__/App.test.tsxui/src/__tests__/menu.test.tsxui/src/api/v1/schema.tsui/src/contexts/DocTabContext.tsxui/src/features/agent/components/ChatInput.tsxui/src/features/agent/components/DocPicker.tsxui/src/features/agent/components/__tests__/ChatMessages.test.tsxui/src/features/agent/components/__tests__/WorkspaceScopedPickers.test.tsxui/src/features/agent/components/messages/ToolCallBadge.tsxui/src/features/agent/hooks/useDocPageContext.tsui/src/features/search/components/SearchResult.tsxui/src/features/search/components/__tests__/SearchResult.test.tsxui/src/hooks/SSEManager.tsui/src/hooks/__tests__/SSEManager.test.tsui/src/hooks/useDocSSE.tsui/src/hooks/useDocTreeSSE.tsui/src/layouts/ContentNavigation.tsxui/src/menu.tsxui/src/pages/__tests__/DocArboristNode.test.tsxui/src/pages/api-docs/ScalarViewer.tsxui/src/pages/api-docs/__tests__/index.test.tsxui/src/pages/api-docs/index.tsxui/src/pages/docs/components/CreateDocModal.tsxui/src/pages/docs/components/DocArboristNode.tsxui/src/pages/docs/components/DocEditor.tsxui/src/pages/docs/components/DocExternalChangeDialog.tsxui/src/pages/docs/components/DocOutlinePanel.tsxui/src/pages/docs/components/DocTabBar.tsxui/src/pages/docs/components/DocTabEditorPanel.tsxui/src/pages/docs/components/DocTreeSidebar.tsxui/src/pages/docs/components/RenameDocModal.tsxui/src/pages/docs/index.tsxui/src/pages/docs/lib/__tests__/doc-mutation.test.tsui/src/pages/docs/lib/__tests__/doc-url.test.tsui/src/pages/docs/lib/__tests__/doc-validation.test.tsui/src/pages/docs/lib/doc-mutation.tsui/src/pages/docs/lib/doc-url.tsui/src/pages/docs/lib/doc-validation.tsui/src/pages/git-sync/MoveDialog.tsxui/src/pages/git-sync/RowActionMenu.tsxui/src/pages/git-sync/index.tsxui/src/pages/home/index.tsxui/src/pages/integrations/__tests__/index.test.tsxui/src/pages/integrations/index.tsxui/src/pages/search/__tests__/index.test.tsxui/src/pages/search/index.tsxui/src/styles/global.cssui/src/types/scalar-api-reference-react.d.ts
💤 Files with no reviewable changes (79)
- ui/src/features/agent/components/tests/WorkspaceScopedPickers.test.tsx
- ui/src/pages/docs/lib/doc-url.ts
- ui/src/pages/integrations/index.tsx
- ui/src/pages/docs/lib/tests/doc-url.test.ts
- ui/src/pages/docs/lib/tests/doc-validation.test.ts
- internal/persis/file/doc/store.go
- internal/agent/types.go
- ui/src/hooks/useDocTreeSSE.ts
- ui/src/pages/integrations/tests/index.test.tsx
- ui/src/pages/docs/lib/tests/doc-mutation.test.ts
- ui/src/hooks/useDocSSE.ts
- internal/cmd/config_test.go
- internal/persis/file/doc/birthtime_darwin.go
- ui/src/pages/docs/components/CreateDocModal.tsx
- ui/src/types/scalar-api-reference-react.d.ts
- internal/cmd/config.go
- ui/src/pages/docs/index.tsx
- internal/core/exec/context.go
- internal/persis/file/doc/birthtime_other.go
- internal/cmn/config/definition.go
- ui/src/pages/docs/components/DocTreeSidebar.tsx
- ui/src/pages/docs/components/DocTabBar.tsx
- ui/src/contexts/DocTabContext.tsx
- ui/src/pages/api-docs/tests/index.test.tsx
- ui/src/features/agent/components/DocPicker.tsx
- ui/src/features/agent/components/messages/ToolCallBadge.tsx
- ui/src/pages/docs/lib/doc-validation.ts
- internal/runtime/builtin/harness/harness.go
- ui/src/features/agent/hooks/useDocPageContext.ts
- ui/src/pages/tests/DocArboristNode.test.tsx
- internal/cmn/config/config.go
- internal/core/exec/context_env.go
- internal/core/spec/builder.go
- ui/src/pages/docs/components/DocExternalChangeDialog.tsx
- ui/src/pages/docs/components/RenameDocModal.tsx
- internal/cmn/config/loader.go
- ui/src/hooks/SSEManager.ts
- internal/cmd/agent.go
- ui/src/pages/api-docs/ScalarViewer.tsx
- ui/src/pages/api-docs/index.tsx
- ui/src/pages/docs/components/DocOutlinePanel.tsx
- ui/src/pages/docs/components/DocTabEditorPanel.tsx
- internal/agent/tool_registry.go
- internal/runtime/eval.go
- internal/agent/navigate.go
- ui/src/pages/docs/components/DocArboristNode.tsx
- ui/src/features/search/components/tests/SearchResult.test.tsx
- ui/src/pages/docs/lib/doc-mutation.ts
- ui/src/tests/menu.test.tsx
- internal/cmn/value/template.go
- internal/agent/runbook_manage.go
- ui/src/pages/docs/components/DocEditor.tsx
- internal/agent/tool_registry_test.go
- internal/cmd/process/frontend_store_factories.go
- internal/runtime/builtin/agentstep/executor.go
- internal/cmn/config/loader_test.go
- internal/service/frontend/persistence.go
- internal/gitsync/state.go
- ui/src/styles/global.css
- internal/persis/file/service_stores.go
- internal/service/frontend/sse/app_stream.go
- ui/package.json
- internal/persis/file/doc/birthtime_linux.go
- ui/src/tests/App.test.tsx
- internal/agent/doc.go
- internal/service/frontend/server_test.go
- internal/service/frontend/sse/types.go
- ui/src/hooks/tests/SSEManager.test.ts
- ui/src/App.tsx
- internal/core/exec/env.go
- internal/service/frontend/api/v1/docs.go
- internal/agent/system_prompt_test.go
- internal/service/frontend/api/v1/docs_test.go
- internal/persis/file/doc/store_test.go
- internal/agent/session.go
- internal/agent/runbook_manage_test.go
- internal/agent/api.go
- internal/service/frontend/api/v1/api.go
- internal/service/frontend/api/v1/search.go
Comment on lines
+10
to
+13
| You have persistent memory across sessions. Save durable facts using the memory tool: user preferences, environment details, tool quirks, and stable conventions. Memory is injected into every turn, so keep it compact and focused on facts that will still matter later. | ||
| Prioritize what reduces future user steering — the most valuable memory is one that prevents the user from having to correct or remind you again. User preferences and recurring corrections matter more than procedural task details. | ||
| Do NOT save task progress, session outcomes, completed-work logs, or temporary TODO state to memory; use session_search to recall those from past transcripts. If you've discovered a new way to do something, solved a problem that could be necessary later, save it as a skill with the skill tool. | ||
| Write memories as declarative facts, not instructions to yourself. 'User prefers concise responses' ✓ — 'Always respond concisely' ✗. 'Project uses pytest with xdist' ✓ — 'Run tests with pytest -n 4' ✗. Imperative phrasing gets re-read as a directive in later sessions and can cause repeated work or override the user's current request. Procedures and workflows belong in workflow definitions, not memory. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search prompt for tool references =="
rg -n -C2 '\b(memory tool|skill tool|search_skills|session_search|read|patch)\b' \
internal/agent/system_prompt.txt
echo
echo "== Search agent files for tool registration / action injection =="
rg -n -C3 '\b(memory|skill|search_skills|Actions|system_prompt)\b' \
internal/agent/tool_registry.go \
internal/agent/api.go \
internal/agent/session.go \
internal/agent/navigate.go \
internal/agent/system_prompt_test.goRepository: dagucloud/dagu
Length of output: 21782
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate system prompt generator and action wiring =="
git ls-files 'internal/agent/*' | sed -n '1,200p'
echo
echo "== Outline likely prompt files =="
for f in internal/agent/system_prompt.go internal/agent/system_prompt_test.go internal/agent/session.go internal/agent/api.go; do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --view expanded || true
echo
fi
done
echo
echo "== Search for memory/skill action definitions across repo =="
rg -n -C3 '\b(memory|skill|search_skills|session_search|Actions|GenerateSystemPrompt|<tools>|tool(s)?\b)' internal . \
--glob '!**/vendor/**'Repository: dagucloud/dagu
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== system prompt generator =="
f=internal/agent/system_prompt.go
[ -f "$f" ] && sed -n '1,260p' "$f"
echo
echo "== tool/action definitions mentioning memory or skills =="
rg -n -C2 '\b(memory tool|skill tool|search_skills|session_search|memory store|LoadGlobalMemory|LoadDAGMemory|save memory|skill)\b' internal/agent \
--glob '!**/*_test.go'
echo
echo "== where .Actions is injected into the prompt =="
rg -n -C3 '\{\{\.Actions\}\}|\bActions\b' internal/agent \
--glob '!**/*_test.go'Repository: dagucloud/dagu
Length of output: 20656
Replace the nonexistent memory/skill tool references
internal/agent/system_prompt.txt:10-13 tells the agent to use a memory tool and a skill tool, but the actual tool set only exposes file-backed memory handling (read/patch) and skill discovery via search_skills and delegate.skills. Update the prompt to use those names so it doesn’t point the model at unavailable actions.
🤖 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/agent/system_prompt.txt` around lines 10 - 13, The system prompt
text incorrectly refers to nonexistent “memory tool” and “skill tool” actions,
which can mislead the agent. Update the wording in the prompt to match the
available mechanisms, using the actual memory and skill interfaces exposed
elsewhere in the agent flow (for example the file-backed memory read/patch path
and skill discovery via search_skills and delegate.skills). Keep the guidance
about what belongs in memory vs skills, but replace the unsupported tool names
with the real symbols so the prompt stays accurate.
Comment on lines
+189
to
+207
| loadMore: async (cursor?: string): Promise<LoadMoreResponse> => { | ||
| const response = await client.GET('/search/dags/{fileName}/matches', { | ||
| params: { | ||
| path: { fileName: result.fileName }, | ||
| query: { | ||
| remoteNode, | ||
| q: query, | ||
| cursor, | ||
| ...dagWorkspaceQuery, | ||
| }, | ||
| }; | ||
| }, | ||
| }); | ||
|
|
||
| return { | ||
| error: response.error?.message || undefined, | ||
| matches: response.data?.matches ?? [], | ||
| hasMore: response.data?.hasMore ?? false, | ||
| nextCursor: response.data?.nextCursor, | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle thrown request failures in loadMore.
response.error covers API-level failures, but a thrown network/client error would currently escape the callback instead of returning a LoadMoreResponse error state. As per coding guidelines, “Use error handling with try/catch for async operations.”
Proposed fix
loadMore: async (cursor?: string): Promise<LoadMoreResponse> => {
- const response = await client.GET('/search/dags/{fileName}/matches', {
- params: {
- path: { fileName: result.fileName },
- query: {
- remoteNode,
- q: query,
- cursor,
- ...dagWorkspaceQuery,
+ try {
+ const response = await client.GET('/search/dags/{fileName}/matches', {
+ params: {
+ path: { fileName: result.fileName },
+ query: {
+ remoteNode,
+ q: query,
+ cursor,
+ ...dagWorkspaceQuery,
+ },
},
- },
- });
+ });
- return {
- error: response.error?.message || undefined,
- matches: response.data?.matches ?? [],
- hasMore: response.data?.hasMore ?? false,
- nextCursor: response.data?.nextCursor,
- };
+ return {
+ error: response.error?.message || undefined,
+ matches: response.data?.matches ?? [],
+ hasMore: response.data?.hasMore ?? false,
+ nextCursor: response.data?.nextCursor,
+ };
+ } catch (error) {
+ return {
+ error:
+ error instanceof Error
+ ? error.message
+ : 'Failed to load more matches',
+ matches: [],
+ hasMore: false,
+ };
+ }
},📝 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
| loadMore: async (cursor?: string): Promise<LoadMoreResponse> => { | |
| const response = await client.GET('/search/dags/{fileName}/matches', { | |
| params: { | |
| path: { fileName: result.fileName }, | |
| query: { | |
| remoteNode, | |
| q: query, | |
| cursor, | |
| ...dagWorkspaceQuery, | |
| }, | |
| }; | |
| }, | |
| }); | |
| return { | |
| error: response.error?.message || undefined, | |
| matches: response.data?.matches ?? [], | |
| hasMore: response.data?.hasMore ?? false, | |
| nextCursor: response.data?.nextCursor, | |
| }; | |
| loadMore: async (cursor?: string): Promise<LoadMoreResponse> => { | |
| try { | |
| const response = await client.GET('/search/dags/{fileName}/matches', { | |
| params: { | |
| path: { fileName: result.fileName }, | |
| query: { | |
| remoteNode, | |
| q: query, | |
| cursor, | |
| ...dagWorkspaceQuery, | |
| }, | |
| }, | |
| }); | |
| return { | |
| error: response.error?.message || undefined, | |
| matches: response.data?.matches ?? [], | |
| hasMore: response.data?.hasMore ?? false, | |
| nextCursor: response.data?.nextCursor, | |
| }; | |
| } catch (error) { | |
| return { | |
| error: | |
| error instanceof Error | |
| ? error.message | |
| : 'Failed to load more matches', | |
| matches: [], | |
| hasMore: false, | |
| }; | |
| } | |
| }, |
🤖 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/search/components/SearchResult.tsx` around lines 189 - 207,
The SearchResult loadMore callback currently only reads response.error, so a
thrown client/network failure from client.GET can escape instead of producing a
LoadMoreResponse error. Update the loadMore async function in SearchResult to
wrap the request in try/catch, keep the existing response.data mapping on
success, and return a LoadMoreResponse with an error string in the catch path so
failures are always surfaced consistently.
Source: Coding guidelines
This was referenced Aug 4, 2026
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.
Closes #2333
Summary:
Testing:
Summary by cubic
Remove the docs/runbooks feature across backend, frontend, API, agent tools, config, SSE, search, and git sync to simplify the product. Also remove the API Docs UI and related dependencies; purge stale references and env/context bindings; and improve enqueue concurrency test reliability.
Refactors
api.yaml) and regenerated clients (api.gen.go,ui/src/api/v1/schema.ts).runbook_managetool; cleaned system prompt and tool registry; updated navigate tool hints.paths.docs_dir,DAG_DOCS_DIR,context.paths.docs_dir); removed SSE doc topic types, watchers, and endpoint mapping.@milkdown/crepe,@milkdown/react,@scalar/api-reference-react,react-arborist).Migration
/search/docs,/search/docs/matches, all/events/docs*SSE endpoints, docs tree/content routes and UI, agentrunbook_managetool.paths.docs_dir; stop usingDAG_DOCS_DIRandcontext.paths.docs_dirin templates or scripts.docs/items; delete or relocate any repo docs you previously synced./docsor/api-docsno longer exist.Written for commit 6f4781f. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Refactor