Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds index ranking metadata, chunked index import and export, SearX compatibility, extension field widgets and uploads, indexed-result state rendering, URL-parameter preferences, and full-width result slots. It also updates search APIs, authorization, settings synchronization, runtime handling, styling, localization, documentation, and release metadata. Sequence Diagram(s)sequenceDiagram
participant SearchRoute
participant Indexer
participant Client
participant ResultUI
SearchRoute->>Indexer: compute index basis
Indexer-->>SearchRoute: return indexed URLs
SearchRoute-->>Client: stream indexedUrls
Client->>ResultUI: mark matching results as indexing
ResultUI-->>Client: render index relations
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
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 `@src/server/utils/server-lifecycle.ts`:
- Around line 20-27: The helper is Proxmox-specific in name but only checks the
generic LXC marker from /run/systemd/container. Update isProxmoxLXCRuntime to
either add an additional Proxmox-only check (for example, a /etc/pve signal)
before returning true, or rename the function and any callers to a generic LXC
name such as isLXCRuntime() so the behavior matches the detection.
🪄 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 Plus
Run ID: 72b35cb9-94a3-4dff-8faf-72aa7b7e67f5
📒 Files selected for processing (2)
package.jsonsrc/server/utils/server-lifecycle.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Collapsed engine performence tab by default on mobile
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/client/utils/search/streaming-search-dom.ts (2)
97-98: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd breakpoint-boundary regression tests.
Cover widths below and at
768pxto lock down the mobile/desktop accordion behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/utils/search/streaming-search-dom.ts` around lines 97 - 98, Add regression tests for the responsive class logic using the breakpoint constant PANEL_LAYOUT_BREAKPOINT, covering a viewport width below 768px and exactly 768px. Verify the streaming-engine panel created by the relevant search-panel setup receives the mobile class below the boundary and the “ open” class at the boundary.Source: Coding guidelines
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the breakpoint in a shared client module.
This utility now depends on a renderer module solely for a constant, reversing the likely dependency direction and potentially creating a circular dependency. Move
PANEL_LAYOUT_BREAKPOINTto a shared constants module and import it from both consumers. As per coding guidelines, shared logic should remain in shared/util modules while UI orchestration stays inclient/modules/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/utils/search/streaming-search-dom.ts` at line 6, Move PANEL_LAYOUT_BREAKPOINT out of the renderer module into a shared constants or utility module, then update both its existing consumers—including the streaming search utility—to import it from that shared location. Remove the renderer-only import and preserve the constant’s value and behavior.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.
Inline comments:
In `@src/styles/_variables.scss`:
- Line 46: Remove the unexpected blank line before the $modal-width declaration
and keep it adjacent to the surrounding Sass variable declarations.
In `@src/styles/components/overrides/_extension-cards.scss`:
- Line 201: In the extension card styling block, add an empty line between the
`@include` flex-between declaration and the background declaration to satisfy
Stylelint’s declaration spacing rule.
---
Nitpick comments:
In `@src/client/utils/search/streaming-search-dom.ts`:
- Around line 97-98: Add regression tests for the responsive class logic using
the breakpoint constant PANEL_LAYOUT_BREAKPOINT, covering a viewport width below
768px and exactly 768px. Verify the streaming-engine panel created by the
relevant search-panel setup receives the mobile class below the boundary and the
“ open” class at the boundary.
- Line 6: Move PANEL_LAYOUT_BREAKPOINT out of the renderer module into a shared
constants or utility module, then update both its existing consumers—including
the streaming search utility—to import it from that shared location. Remove the
renderer-only import and preserve the constant’s value and behavior.
🪄 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 Plus
Run ID: 71f00929-e898-44da-b27e-e260d92ebbaa
📒 Files selected for processing (6)
src/client/modules/renderer/render-media.tssrc/client/utils/search/streaming-search-dom.tssrc/styles/_variables.scsssrc/styles/components/overrides/_extension-cards.scsssrc/styles/components/overrides/_settings.scsssrc/styles/style.scss
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/routes/search-stream.ts (1)
226-281: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFinalize the SSE stream on errors
Promise.all(enginePromises).then(async () => { ... })has no rejection path, and the awaited work inside can throw before_send("done")/controller.close(). Wrap this finalizer intry/finallyor add a.catchso the stream always terminates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/routes/search-stream.ts` around lines 226 - 281, Update the Promise.all(enginePromises) finalization callback to handle rejections from both engine promises and its awaited work, ensuring controller.close() and stream termination occur even when scoring, settings, indexing, caching, or _send("done") fails. Preserve the existing successful completion payload while adding a rejection/finally path around the finalizer.
🧹 Nitpick comments (2)
src/server/routes/settings.ts (1)
219-222: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid redundant settings re-read in
_reconcileIndexerQueue.
_reconcileIndexerQueuecallsgetInstanceSettings()to re-read settings that were just persisted by the caller. In the/api/settings/fieldhandler, the coerced value is already known. Consider passing the enabled state as a parameter to avoid an extra disk read on every settings update.♻️ Proposed refactor
-const _reconcileIndexerQueue = async (): Promise<void> => { - const settings = await getInstanceSettings(); - if (asBoolean(settings.degoogIndexerEnabled)) await startQueue(); - else await stopQueue(); -}; +const _reconcileIndexerQueue = async (enabled: boolean): Promise<void> => { + if (enabled) await startQueue(); + else await stopQueue(); +};Then at call sites:
- await _reconcileIndexerQueue(); + await _reconcileIndexerQueue(asBoolean(updates.degoogIndexerEnabled));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/routes/settings.ts` around lines 219 - 222, Update _reconcileIndexerQueue to accept the already-coerced degoogIndexerEnabled state as a parameter instead of calling getInstanceSettings(). In the /api/settings/field handler, pass that known value after persisting the setting, while preserving the existing startQueue/stopQueue behavior.src/server/indexer/queue/queue.ts (1)
40-41: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider caching indexer config to avoid reading it on every flush.
getIndexerConfig()is called inside the mutex on everyflushTypeinvocation. If this reads from disk, it adds I/O latency to a hot path that runs frequently. Consider caching the config (with invalidation on settings update) or readingrankingWindowonce perflushQueuecycle and passing it down.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/indexer/queue/queue.ts` around lines 40 - 41, Update the flush flow around flushType and flushQueue to avoid calling getIndexerConfig on every flush while holding the mutex. Read rankingWindow once per flushQueue cycle or cache it with invalidation when settings change, then pass the resolved value into flushType for writeBatch; preserve current behavior when configuration is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/settings/indexer/download.ts`:
- Around line 86-92: Update the export flow around openWriter so picker
cancellation still closes the server-side session identified by sessionId. Move
destination opening into the existing try/finally cleanup scope, or explicitly
invoke the export-end cleanup before returning when writer is null, while
preserving normal writer handling.
- Around line 99-108: Update the chunk download loop around the response
handling and writer finalization to verify each response contains exactly the
requested end-pos byte count before writing it. On any non-success response or
incomplete chunk, abort the writable stream and propagate or return the failure
without calling writer.close(); only finalize the file with writer.close() after
all chunks pass validation.
In `@src/client/settings/indexer/export-modal.ts`:
- Around line 78-84: Update the type-selector construction in the export modal
to avoid interpolating type metadata into innerHTML. Create the select and each
option with DOM APIs, assigning option.value and option.textContent from type,
then append the selector into the existing degoog-select-wrap while preserving
the export-modal-desc rendering.
In `@src/client/settings/indexer/import-modal.ts`:
- Around line 146-185: Update the saveEl click listener in the import modal so
validation or runImport failures leave the handler available for another
attempt. Remove the one-time listener behavior from the initial addEventListener
call, while preserving the existing button state restoration and the separate
one-time close listener after a successful import.
In `@src/client/settings/indexer/manage-modal.ts`:
- Around line 20-24: Update the select-all input and each row-selection checkbox
in the indexer management modal to include localized aria-label values
describing their actions. Use the existing translation/localization mechanism
and ensure both the input identified by indexer-manage-selectall and the row
checkbox inputs are covered.
In `@src/locales/it.json`:
- Line 443: Update the "import-processing" translation in it.json to use the
properly accented Italian “può” instead of the apostrophe spelling “puo'”,
leaving the rest of the message unchanged.
In `@src/server/indexer/adapters/postgres/adapter.ts`:
- Around line 109-123: The _ensureHitsColumns method currently exits when
pos_sum exists, preventing missing companion columns from being added. Query the
presence of all four required query_hits columns independently, add each absent
column without an early return, and only perform the pos_sum backfill as needed
after the required columns are ensured.
In `@src/server/indexer/import/importer.ts`:
- Around line 64-66: Update both reader catch blocks in the importer flow to log
the failure and rethrow a sanitized import error instead of returning an empty
array. Ensure importFromFile propagates this error so corrupt or unsupported
files do not produce a successful zero-count result; preserve normal row-reading
behavior for valid files.
- Around line 58-76: Update readSqliteRows and readSqlRows to treat parsed
database results as unknown and validate each imported row’s required fields,
timestamps, URLs, and JSON metadata before returning it as ExportRow[]. Reuse or
add a row-validation symbol near these readers, filter or reject invalid rows
consistently, and ensure only validated rows reach the adapter and persistence
flow.
- Around line 47-52: Refactor the import pipeline to avoid loading complete
files or row sets into memory: update isSqliteFile to read only the SQLite
header, replace SQLite .all() usage with paged row retrieval, and change
parseSqlDump/import processing to yield or process SQL statements incrementally.
Preserve flushRows batching while ensuring rows are flushed during traversal
rather than after the full upload or result set is materialized.
In `@src/server/indexer/import/sql-parser.ts`:
- Around line 30-37: Update unescapeCopy to decode COPY escapes in a single
left-to-right pass, treating each backslash as an escape introducer and mapping
\t, \n, \r, and \\ appropriately. Remove the order-dependent sequential
replacements so encoded literal backslashes adjacent to letters, such as
C:\\next, remain literal and are not misinterpreted as newline escapes.
In `@src/server/indexer/recorders/default.ts`:
- Line 83: Update the position assignment in the recorder mapping to fall back
to the item index when positions is absent or positions[i] is undefined.
Preserve valid entries from the positions array while ensuring position always
receives a numeric value.
In `@src/server/indexer/store/hits.ts`:
- Line 27: Update the sort comparator in the hits listing to handle null or
undefined query_norm values without throwing. Use a consistent null-safe
fallback for query_norm before calling localeCompare, while preserving the
existing score-based tie-breaker and ordering for valid strings.
In `@src/server/indexer/transfer/sessions.ts`:
- Around line 45-60: Update sweep and the session access paths around
open*Session to reject any session whose expires timestamp has passed, even when
no new session is created. Schedule sweep to run periodically so abandoned
export/import entries are cleaned up, including ending import sinks and removing
their files, while preserving cleanup of expired sessions during normal access
and creation flows.
In `@src/server/routes/search/_search-handlers.ts`:
- Around line 180-193: Resolve the interceptor-overridden searchType, lang, and
timeFilter before constructing filters or selecting the indexing path in the
search handler around maybeIndex. Reuse those resolved values for retry
execution, cache-key construction, engine selection, and the toFilterTag call,
matching handleSearch behavior instead of the original request values.
In `@src/server/routes/settings.ts`:
- Around line 219-224: Update _reconcileIndexerQueue so the enabled branch
awaits startQueue(), matching the existing await stopQueue() behavior. Preserve
the current setting check and queue start/stop branching while ensuring
initialization completes and any rejection propagates before the function
resolves.
In `@tests/indexer/engine-selection.test.ts`:
- Around line 37-50: Clean up the index-web.db marker created by the
“auto-enables degoog engine for known web index type when no explicit config”
test. Use a finally block around its assertions to remove the marker, or reset
the shared filesystem in beforeEach, so the “does not surface degoog engine”
test remains order-independent.
In `@tests/indexer/transfer.test.ts`:
- Around line 6-10: Update the shared test-directory setup around SHARED to
create a unique directory with mkdtempSync instead of using the fixed
degoog-indexer-tests path, and add afterAll cleanup that removes it recursively.
Keep the existing environment-variable paths based on SHARED while ensuring
stale test artifacts cannot persist between runs.
---
Outside diff comments:
In `@src/server/routes/search-stream.ts`:
- Around line 226-281: Update the Promise.all(enginePromises) finalization
callback to handle rejections from both engine promises and its awaited work,
ensuring controller.close() and stream termination occur even when scoring,
settings, indexing, caching, or _send("done") fails. Preserve the existing
successful completion payload while adding a rejection/finally path around the
finalizer.
---
Nitpick comments:
In `@src/server/indexer/queue/queue.ts`:
- Around line 40-41: Update the flush flow around flushType and flushQueue to
avoid calling getIndexerConfig on every flush while holding the mutex. Read
rankingWindow once per flushQueue cycle or cache it with invalidation when
settings change, then pass the resolved value into flushType for writeBatch;
preserve current behavior when configuration is unchanged.
In `@src/server/routes/settings.ts`:
- Around line 219-222: Update _reconcileIndexerQueue to accept the
already-coerced degoogIndexerEnabled state as a parameter instead of calling
getInstanceSettings(). In the /api/settings/field handler, pass that known value
after persisting the setting, while preserving the existing startQueue/stopQueue
behavior.
🪄 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 Plus
Run ID: c78ca371-05d0-41fa-960b-f71ed9a7a10b
📒 Files selected for processing (63)
package.jsonsrc/client/modules/renderer/render-sidebar.tssrc/client/modules/renderer/render.tssrc/client/modules/settings/settings.tssrc/client/settings/indexer/api.tssrc/client/settings/indexer/download.tssrc/client/settings/indexer/export-modal.tssrc/client/settings/indexer/import-modal.tssrc/client/settings/indexer/manage-modal.tssrc/client/settings/indexer/progress.tssrc/client/settings/indexer/public.tssrc/client/settings/indexer/shell.tssrc/client/settings/indexer/tab.tssrc/client/settings/indexer/toggles.tssrc/client/utils/search/streaming-search-dom.tssrc/client/utils/streaming-search.tssrc/locales/en-US.jsonsrc/locales/fr-FR.jsonsrc/locales/he.jsonsrc/locales/it.jsonsrc/public/settings-public.htmlsrc/public/themes/degoog-theme/locales/en-US.jsonsrc/public/themes/degoog-theme/locales/fr-FR.jsonsrc/public/themes/degoog-theme/locales/he.jsonsrc/public/themes/degoog-theme/locales/it.jsonsrc/public/themes/degoog-theme/search-templates/result.htmlsrc/server/indexer/adapters/postgres/adapter.tssrc/server/indexer/adapters/postgres/schema.tssrc/server/indexer/adapters/sqlite/adapter.tssrc/server/indexer/adapters/sqlite/statements.tssrc/server/indexer/config/load.tssrc/server/indexer/export/builder.tssrc/server/indexer/export/schema.tssrc/server/indexer/import/importer.tssrc/server/indexer/import/sql-parser.tssrc/server/indexer/queue/queue.tssrc/server/indexer/recorders/default.tssrc/server/indexer/shared/export-select.tssrc/server/indexer/shared/rank-fields.tssrc/server/indexer/store/hits.tssrc/server/indexer/store/index.tssrc/server/indexer/store/mapper.tssrc/server/indexer/store/record.tssrc/server/indexer/transfer/sessions.tssrc/server/indexer/types/adapter.tssrc/server/indexer/types/config.tssrc/server/routes/indexer.tssrc/server/routes/search-stream.tssrc/server/routes/search/_search-handlers.tssrc/server/routes/settings.tssrc/server/search.tssrc/server/utils/paths.tssrc/server/utils/settings-schema.tssrc/shared/indexer.tssrc/shared/search-types.tssrc/styles/components/_generic.scsssrc/styles/components/generic/_progress.scsssrc/styles/components/generic/_typography.scsstests/indexer/engine-selection.test.tstests/indexer/routes.test.tstests/indexer/sql-parser.test.tstests/indexer/store.test.tstests/indexer/transfer.test.ts
💤 Files with no reviewable changes (4)
- src/client/settings/indexer/public.ts
- src/public/settings-public.html
- src/client/settings/indexer/tab.ts
- src/client/modules/settings/settings.ts
…beling Remove insecure --security-opt label=disable flag
Fixed issue with dev versioning
Fixed compilation issues
|
@coderabbitai please can you write ONE message with all actionable prompts from the various REAL issues in this PR? All I need is file:line affected and all prompts for llm in ONE message. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="degoog-org/degoog"
pr="226"
echo "=== REVIEW COMMENTS ==="
gh api --paginate "repos/$repo/pulls/$pr/comments?per_page=100" \
--jq '.[] | {
id,
user: .user.login,
path,
line,
original_line,
side,
body,
created_at,
in_reply_to_id
}'
echo "=== TOP-LEVEL PR COMMENTS ==="
gh api --paginate "repos/$repo/issues/$pr/comments?per_page=100" \
--jq '.[] | select(.user.login == "coderabbitai" or (.body | test("CodeRabbit"; "i"))) | {
id,
user: .user.login,
body,
created_at
}'
echo "=== REVIEW THREADS ==="
gh api graphql \
-F owner="degoog-org" \
-F name="degoog" \
-F pr="$pr" \
-f query='
query($owner: String!, $name: String!, $pr: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $pr) {
reviewThreads(first: 100) {
nodes {
isResolved
path
line
originalLine
comments(first: 20) {
nodes {
databaseId
author { login }
body
createdAt
}
}
}
}
}
}
}' \
--jq '.data.repository.pullRequest.reviewThreads.nodes'Length of output: 246023
You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/server/extensions/compatibility-layer/searx/bridge/runtime.py (1)
103-104: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winResolve the new Ruff violations without changing fallback behavior.
The blanket silent handler triggers Ruff S110 and BLE001. Catch the expected RPC failures and return the local value.
Proposed fix
- except Exception: - pass + except (OSError, RuntimeError, ValueError): + return value🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/extensions/compatibility-layer/searx/bridge/runtime.py` around lines 103 - 104, Update the exception handling in the surrounding runtime bridge method to catch only the expected RPC failure types instead of blanket Exception, and return the local fallback value from that handler. Preserve the existing fallback behavior while eliminating the silent broad exception handling that triggers Ruff S110 and BLE001.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/extensions/compatibility-layer/searx/api-shape.ts`:
- Around line 112-114: Update the catch block in the URL parsing logic to remove
the raw URL and parser error from logger.debug, emitting only a generic failure
message while preserving the existing fallback return value.
In `@src/server/extensions/compatibility-layer/searx/index.ts`:
- Around line 176-180: Sanitize the untrusted req.url value in the onFetch
handler before interpolating it into logger.warn, stripping or encoding CR/LF
and other control characters while preserving the existing blocked-request
behavior and error message.
In `@src/server/routes/search/_search-routes.ts`:
- Line 106: Update the form POST response call in the search route to use the
query format as a fallback when form.get(SEARX_FORMAT_PARAM) is absent, matching
the existing body-first, query-second precedence used by JSON POST requests. Add
a regression test covering a form POST with ?format=json and assert the
observable JSON response behavior.
In `@src/server/routes/settings.ts`:
- Around line 220-226: Update the settings save flow and _reloadSearx so
reloadEngines failures are observable when searxCompatEnabled changes instead of
being swallowed by the warning log. Propagate the failure or return an explicit
retry/error state from POST /api/settings.general, while preserving successful
saves and unchanged-toggle behavior; add a route test covering rejected
reloadEngines.
In `@tests/routes/extensions-upload.test.ts`:
- Around line 62-65: Update the environment setup and cleanup around the
affected test suite so the original values of DEGOOG_PUBLIC_INSTANCE and
DEGOOG_SETTINGS_PASSWORDS are captured before deletion and restored in afterAll,
alongside DEGOOG_DANGEROUSLY_NO_PASSWORD; preserve unset variables as unset.
---
Duplicate comments:
In `@src/server/extensions/compatibility-layer/searx/bridge/runtime.py`:
- Around line 103-104: Update the exception handling in the surrounding runtime
bridge method to catch only the expected RPC failure types instead of blanket
Exception, and return the local fallback value from that handler. Preserve the
existing fallback behavior while eliminating the silent broad exception handling
that triggers Ruff S110 and BLE001.
🪄 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 Plus
Run ID: 9f4df3db-1cac-4e86-b079-22e50d988920
📒 Files selected for processing (29)
src/client/modules/init.tssrc/client/settings/engines/searx-api.tssrc/client/settings/engines/tab.tssrc/client/settings/indexer/import-modal.tssrc/client/settings/server/auto-save.tssrc/client/settings/server/render.tssrc/client/settings/server/tab.tssrc/client/types/settings-server.tssrc/locales/en-US.jsonsrc/server/extensions/compatibility-layer/searx/api-shape.tssrc/server/extensions/compatibility-layer/searx/bridge/engine.pysrc/server/extensions/compatibility-layer/searx/bridge/runtime.pysrc/server/extensions/compatibility-layer/searx/index.tssrc/server/extensions/compatibility-layer/searx/install.tssrc/server/extensions/compatibility-layer/searx/rpc.tssrc/server/indexer/adapters/sqlite/adapter.tssrc/server/indexer/import/importer.tssrc/server/indexer/import/sql-parser.tssrc/server/routes/extensions.tssrc/server/routes/search/_search-routes.tssrc/server/routes/searx-engines.tssrc/server/routes/settings.tssrc/server/types/search.tssrc/server/utils/settings-schema.tstests/indexer/sql-parser.test.tstests/indexer/sqlite-migrate.test.tstests/routes/extensions-upload.test.tstests/unit/searx-api-shape.test.tstests/unit/searx-compat.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/client/modules/init.ts
- src/server/utils/settings-schema.ts
- src/client/settings/engines/tab.ts
- src/server/routes/extensions.ts
- tests/unit/searx-compat.test.ts
- src/server/extensions/compatibility-layer/searx/rpc.ts
- src/server/indexer/import/importer.ts
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/server/routes/search-stream.ts (1)
161-168: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRemove the unused
displayResultscomputation and reuse the settings already read.
displayResultsat Line 165 is never used after finalization, andrawScoredResultsat Line 162 exists only to build it.applyDomainRulesperforms async domain lookups, so this is wasted work on every stream close. Line 164 also re-reads instance settings that Line 44 already loaded intosettings.♻️ Proposed cleanup
const totalTime = Math.round(performance.now() - start); - const rawScoredResults = scoreResults(allRawResults); - - const indexerSettings = await getInstanceSettings(); - const displayResults = await applyDomainRules(rawScoredResults); const indexBasis = await applyDomainRules( scoreResults(allRawResults.filter((e) => e.name !== DEGOOG_ENGINE_NAME)), );Then update the
maybeIndexcall to usesettings:const indexedUrls = await maybeIndex( - asBoolean(indexerSettings.degoogIndexerEnabled), + asBoolean(settings.degoogIndexerEnabled),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/routes/search-stream.ts` around lines 161 - 168, In the stream finalization flow, remove the unused rawScoredResults, displayResults, and indexerSettings computations around scoreResults/applyDomainRules. Reuse the existing settings value loaded earlier instead of calling getInstanceSettings again, and update the maybeIndex call to receive settings.src/server/routes/search/_search-handlers.ts (2)
108-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the run-collection step from
handleRetry.
handleRetrynow spans roughly 105 lines and mixes override resolution, fresh engine execution, cache reads, live fallback searches, scoring, indexing, and thumbnail signing. Lines 108-150 form one cohesive unit: collect the runs for the non-retried engines and merge them with the fresh run. Move that unit into a helper insrc/server/search/engine-cache.tsor a sibling module, and keephandleRetryas the orchestrator.As per coding guidelines, "Suggest splitting functions that grow to roughly 60+ lines or that mix parsing, validation, persistence, rendering, and logging."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/routes/search/_search-handlers.ts` around lines 108 - 150, Extract the run-collection and merge logic from handleRetry into a focused helper in engine-cache.ts or a sibling module. The helper should accept the resolved search scope, active engines, retried engine identity, fresh results, and timing, then perform readActiveRuns, live searchSingleEngine calls, scoreResults merging, and engine timing aggregation; leave handleRetry responsible only for orchestration and consuming the helper’s result.Source: Coding guidelines
152-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the index-and-sign tail between
handleSearchandhandleRetry.Lines 152-178 duplicate Lines 47-69 almost exactly: read settings, build
filtersTag, callmaybeIndex, thentagIndexRelationandsignResultThumbnails. The two copies already differ in how they exclude recalled results, and further drift changes indexing behavior on only one path. Extract one helper that takes the display results, the index candidates, and the resolved filter context.The retry response shape at Lines 169-179 also differs from the
handleSearchshape. Declare both shapes as named DTOs insrc/sharedso the client contract stays explicit.As per coding guidelines, "Use strict DTOs for data crossing boundaries (client/server, route/orchestration, registry/extensions), and place shared shapes in
src/sharedto avoid drift."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/routes/search/_search-handlers.ts` around lines 152 - 178, Extract the duplicated settings, filter-tag, indexing, relation-tagging, and thumbnail-signing flow from handleSearch and handleRetry into one helper accepting display results, index candidates, and resolved filter context; preserve each path’s existing recalled-result exclusion when building candidates. Define named shared DTOs for both response shapes in src/shared, then update both handlers to use those DTOs and the shared tail helper.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.
Inline comments:
In `@src/client/settings/engines/searx-modal.ts`:
- Around line 11-15: Add the missing searx-updating and searx-updated locale
entries to the fr-FR, he, it, and degoog-theme locale/theme resources, using the
existing searx translation structure and appropriate translated values so the
modal’s busy and completion statuses never display raw keys.
In `@src/server/search.ts`:
- Around line 313-324: Wrap the cache-key generation and cache-read logic in
searchSingleEngine with the existing try/error-isolation path so failures from
runKey or readRun fall back to normal engine execution and return a
CachedEngineRun rather than rejecting the search. Apply the same best-effort
handling in the corresponding search-stream flow, and only perform cache writes
when a valid key was successfully produced.
In `@src/server/utils/cache.ts`:
- Around line 194-197: Update the engine run cache flow around engineRunCache
and readRun so cache hits are validated as CachedEngineRun objects before being
returned or consumed by src/server/search.ts. Prevent legacy or colliding
entries under SEARCH_NAMESPACE from reaching code that accesses
hit.timing.resultCount, using either a dedicated run-cache namespace or a shape
check in readRun while preserving valid cache hits.
In `@tests/unit/search.test.ts`:
- Line 3: Expand the cache-key tests in engine-selection-score-cache.test.ts to
verify that changing lang, dateFrom, dateTo, and each remaining image-filter
dimension changes the key. Add a separate assertion for an overriding engine
configuration that changes only that engine’s runKey, confirming the override
affects its cache key without requiring unrelated setting changes.
---
Nitpick comments:
In `@src/server/routes/search-stream.ts`:
- Around line 161-168: In the stream finalization flow, remove the unused
rawScoredResults, displayResults, and indexerSettings computations around
scoreResults/applyDomainRules. Reuse the existing settings value loaded earlier
instead of calling getInstanceSettings again, and update the maybeIndex call to
receive settings.
In `@src/server/routes/search/_search-handlers.ts`:
- Around line 108-150: Extract the run-collection and merge logic from
handleRetry into a focused helper in engine-cache.ts or a sibling module. The
helper should accept the resolved search scope, active engines, retried engine
identity, fresh results, and timing, then perform readActiveRuns, live
searchSingleEngine calls, scoreResults merging, and engine timing aggregation;
leave handleRetry responsible only for orchestration and consuming the helper’s
result.
- Around line 152-178: Extract the duplicated settings, filter-tag, indexing,
relation-tagging, and thumbnail-signing flow from handleSearch and handleRetry
into one helper accepting display results, index candidates, and resolved filter
context; preserve each path’s existing recalled-result exclusion when building
candidates. Define named shared DTOs for both response shapes in src/shared,
then update both handlers to use those DTOs and the shared tail helper.
🪄 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 Plus
Run ID: a5faf04c-c140-4d57-9fe7-8c183328ca6e
📒 Files selected for processing (18)
src/client/settings/engines/searx-api.tssrc/client/settings/engines/searx-modal.tssrc/client/settings/engines/searx-render.tssrc/locales/en-US.jsonsrc/server/extensions/compatibility-layer/searx/install.tssrc/server/routes/search-stream.tssrc/server/routes/search/_lucky-route.tssrc/server/routes/search/_search-handlers.tssrc/server/routes/searx-engines.tssrc/server/search.tssrc/server/search/engine-cache.tssrc/server/search/engine-selection.tssrc/server/utils/cache.tssrc/server/utils/search.tstests/unit/cache.test.tstests/unit/engine-selection-score-cache.test.tstests/unit/search.test.tstests/unit/searx-install.test.ts
💤 Files with no reviewable changes (1)
- src/server/utils/search.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/client/settings/engines/searx-render.ts
- src/locales/en-US.json
- src/client/settings/engines/searx-api.ts
| const BUSY_KEYS: Record<SearxAction, string> = { | ||
| [SearxAction.Install]: "settings-page.extensions.searx-installing", | ||
| [SearxAction.Update]: "settings-page.extensions.searx-updating", | ||
| [SearxAction.Uninstall]: "settings-page.extensions.searx-uninstalling", | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the new SearX status keys across all locale files.
fd -t f -e json . src/locales src/public/themes | while IFS= read -r f; do
for k in searx-updating searx-updated searx-installing searx-uninstalling searx-restart; do
rg -q "\"$k\"" "$f" || echo "missing $k in $f"
done
doneRepository: degoog-org/degoog
Length of output: 2722
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -t f 'searx-modal\.ts$|.*searx.*\.(json|ts|tsx)$|^src/(locales|public/themes/[^/]+/locales|public/themes/[^/]+/theme)\.json$' . | sed 's#^\./##' | sort
echo
echo "== searx status key usage in searx-modal.ts =="
file="$(fd -t f 'searx-modal\.ts$' src/client/settings/engines | head -n 1)"
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '1,120p' "$file" | cat -n
fi
echo
echo "== locale key definitions for settings-page.extensions.searx-* =="
rg -n '"settings-page"\s*:' -S src/locales src/public/themes | head -n 50
for pattern in 'src/locales/*.json' 'src/public/themes/degoog-theme/locales/*.json' 'src/public/themes/degoog-theme/theme.json'; do
echo "-- $pattern --"
rg -n 'searx-(updating|updated|installing|uninstalling|restart)' $pattern || true
doneRepository: degoog-org/degoog
Length of output: 5953
Add the missing SearX status locale keys.
BUSY_KEYS now uses settings-page.extensions.searx-updating, and the modal also uses settings-page.extensions.searx-updated; add these keys for fr-FR, he, it, and the degoog-theme locale/theme files so translators do not render raw keys in the busy or completion status.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/settings/engines/searx-modal.ts` around lines 11 - 15, Add the
missing searx-updating and searx-updated locale entries to the fr-FR, he, it,
and degoog-theme locale/theme resources, using the existing searx translation
structure and appropriate translated values so the modal’s busy and completion
statuses never display raw keys.
Hi all, The next release of degoog will allow running 80+ searx engines natively running a python runner on demand when the engine is called. The way it works is on the lines of: ``` -> user runs a search -> degoog runs the compatibility engine -> spawn a python runner which build a faux SearX request -> degoog fetches URL through normal engine context like it would for any typescript engine -> spawn a python runner that parses response with the SearX response() method -> normalize to Degoog results ``` The only issue with this is that the machine will need to have specific python library installed (it's the only trade off from this approach). I figured I'd just open a PR to help out as somebody likely will request this once it goes live ❤️ I can't test this locally as I do not have proxmox running but it's basic apt install, it'd be great to make sure it compiles however 😆 These are the release note and the open release PR. Still fixing some bugs but should be released within the week: degoog-org/degoog#226
|
@coderabbitai want to summarise any action that's still relevant and write it out in ONE long prompt? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="degoog-org/degoog"
pr="226"
echo "=== PR HEAD ==="
gh pr view "$pr" --repo "$repo" --json headRefOid,updatedAt,url \
--jq '{headRefOid, updatedAt, url}'
echo "=== UNRESOLVED CODERABBIT REVIEW THREADS ==="
gh api graphql \
-F owner="degoog-org" \
-F name="degoog" \
-F pr="$pr" \
-f query='
query($owner: String!, $name: String!, $pr: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $pr) {
reviewThreads(first: 100) {
nodes {
id
isResolved
path
line
originalLine
comments(first: 30) {
nodes {
databaseId
author { login }
body
createdAt
}
}
}
}
}
}
}' --jq '
.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| select(any(.comments.nodes[]; .author.login == "coderabbitai"))
| {
id, path, line, originalLine,
comments: [.comments.nodes[] | {
databaseId, author: .author.login, body, createdAt
}]
}'
echo "=== RELEVANT SOURCE MAPS ==="
for file in \
src/client/modules/init.ts \
src/client/modules/renderer/render-slots.ts \
src/client/settings/indexer/import-modal.ts \
src/client/utils/template.ts \
src/server/indexer/adapters/sqlite/adapter.ts \
src/server/indexer/import/sql-parser.ts \
src/server/routes/extensions.ts \
src/server/extensions/compatibility-layer/searx/bridge/engine.py \
src/server/extensions/compatibility-layer/searx/bridge/runtime.py \
src/server/extensions/compatibility-layer/searx/index.ts \
src/server/extensions/compatibility-layer/searx/install.ts \
src/server/extensions/compatibility-layer/searx/rpc.ts \
src/server/routes/searx-engines.ts
do
if [ -f "$file" ]; then
echo "--- $file ---"
ast-grep outline "$file" --items all 2>/dev/null || true
fi
done
echo "=== TARGETED IMPLEMENTATION MARKERS ==="
rg -n -C 4 \
'HIDE_URL_PARAMS|performSearch|innerHTML|sanitize|DOMPurify|runImport|onSave|addEventListener|removeEventListener|`@index`|pos_sum|user_version|transaction|split.*;|MAX.*SQL|bodyLimit|maxSizeKb|arrayBuffer|searx-desc|base_url|ttl_seconds|secret_hash|hashlib|new URL|redirect|withSearxLock|\\.tmp|flush\\(|Bun\\.spawn|timeout|kill' \
src/client/modules/init.ts \
src/client/modules/renderer/render-slots.ts \
src/client/settings/indexer/import-modal.ts \
src/client/utils/template.ts \
src/server/indexer/adapters/sqlite/adapter.ts \
src/server/indexer/import/sql-parser.ts \
src/server/routes/extensions.ts \
src/locales/en-US.json \
src/server/extensions/compatibility-layer/searx \
src/server/routes/searx-engines.ts || trueLength of output: 58730 🏁 Script executed: #!/bin/bash
set -euo pipefail
show() {
local file="$1" start="$2" end="$3"
echo
echo "===== $file:$start-$end ====="
sed -n "${start},${end}p" "$file" | cat -n
}
show src/client/modules/renderer/render-slots.ts 1 100
show src/client/utils/template.ts 1 80
show src/server/indexer/import/sql-parser.ts 110 340
show src/server/routes/extensions.ts 45 115
show src/server/routes/extensions.ts 330 430
show src/server/extensions/compatibility-layer/searx/install.ts 20 215
show src/server/extensions/compatibility-layer/searx/api-shape.ts 85 130
show src/server/extensions/compatibility-layer/searx/index.ts 145 210
show src/server/extensions/compatibility-layer/searx/rpc.ts 75 210
show src/server/routes/search/_search-routes.ts 70 125
show src/server/routes/settings.ts 190 290
show tests/routes/extensions-upload.test.ts 1 240
show src/client/settings/engines/searx-modal.ts 1 140
show src/server/search.ts 285 380
show src/server/routes/search-stream.ts 120 200
show src/server/search/engine-cache.ts 1 160
show src/server/utils/cache.ts 160 220
echo
echo "===== LOCALE STATUS KEYS ====="
rg -n -C 2 '"searx-(updating|updated|installing|uninstalling|restart)"' \
src/locales src/public/themes/degoog-theme 2>/dev/null || true
echo
echo "===== FOCUSED REGRESSION TESTS ====="
rg -n -C 3 \
'FullWidthAboveResults|slot-full-width|renderSlotPanels|renderTemplate|`#each`|splitStatements|semicolon|SQL.*(size|dump)|extensions.*upload|DEGOOG_PUBLIC_INSTANCE|searx.*(install|lock|format|reload|URL)|runKey|readRun|CachedEngineRun|cache read' \
tests src --glob '*.{test.ts,test.tsx}' 2>/dev/null || trueLength of output: 91715 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "===== SQL DUMP SIZE ENFORCEMENT ====="
rg -n -C 5 'MAX.*SQL|SQL.*MAX|256|parseSqlDump|\\.sql|stat\\(|size' \
src/server/indexer/import src/server/routes/indexer.ts
echo
echo "===== SEARX MUTATION LOCK SCOPE ====="
sed -n '1,150p' src/server/routes/searx-engines.ts | cat -n
rg -n -C 4 '\b(installSearx|updateSearx|uninstallSearx|withSearxLock)\b' \
src tests --glob '*.{ts,tsx}'
echo
echo "===== RPC TEST COVERAGE ====="
rg -n -C 3 'runPython|stdin|flush|timeout|no.*envelope|kill' \
tests --glob '*.{ts,tsx}' || trueLength of output: 1022 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "===== SQL DUMP SIZE ENFORCEMENT ====="
rg -n -C 5 -e 'parseSqlDump' -e '256' -e '\.sql' -e 'stat\(' -e '\.size\b' \
src/server/indexer/import src/server/routes/indexer.ts || true
echo
echo "===== SEARX MUTATION LOCK SCOPE ====="
sed -n '1,150p' src/server/routes/searx-engines.ts | cat -n
rg -n -C 4 -e 'installSearx' -e 'updateSearx' -e 'uninstallSearx' -e 'withSearxLock' \
src tests --glob '*.{ts,tsx}' || true
echo
echo "===== RPC TEST COVERAGE ====="
rg -n -C 3 -e 'runPython' -e 'stdin' -e 'flush' -e 'timeout' -e 'envelope' -e '\.kill\(' \
tests --glob '*.{ts,tsx}' || trueLength of output: 45532 🏁 Script executed: #!/bin/bash
set -euo pipefail
for thread_id in \
PRRT_kwDORejnm86TpP-0 \
PRRT_kwDORejnm86TpP-4 \
PRRT_kwDORejnm86Vmyv8
do
gh api graphql \
-F threadId="$thread_id" \
-f query='
mutation($threadId: ID!) {
resolveReviewThread(input: {threadId: $threadId}) {
thread { id isResolved }
}
}' \
--jq '.data.resolveReviewThread.thread | {id, isResolved}'
doneLength of output: 666
Use this single prompt for the remaining valid work: You are interacting with an AI system. |
…a little as they were becoming a fucking mess
|
@coderabbitai one last time, triple check everything, I have pushed my changes and it should probably close all the pending comments, if that's not the case let me know, I did purposely ignore 1 and 8. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="degoog-org/degoog"
pr="226"
echo "=== PR HEAD ==="
gh pr view "$pr" --repo "$repo" --json headRefOid,updatedAt,url --jq '{headRefOid,updatedAt,url}'
echo "=== UNRESOLVED CODERABBIT THREADS ==="
gh api graphql \
-F owner="degoog-org" -F name="degoog" -F pr="$pr" \
-f query='
query($owner: String!, $name: String!, $pr: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $pr) {
reviewThreads(first: 100) {
nodes {
id isResolved path line originalLine
comments(first: 30) {
nodes { author { login } body createdAt }
}
}
}
}
}
}' --jq '
.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| select(any(.comments.nodes[]; .author.login == "coderabbitai"))
| {id,path,line,originalLine,comments:[.comments.nodes[]|{author:.author.login,body,createdAt}]}'
echo "=== SOURCE OUTLINES ==="
for f in \
src/client/modules/renderer/render-slots.ts \
src/client/utils/template.ts \
src/server/extensions/compatibility-layer/searx/api-shape.ts \
src/server/extensions/compatibility-layer/searx/index.ts \
src/server/routes/search/_search-routes.ts \
src/server/routes/settings.ts \
tests/routes/extensions-upload.test.ts \
src/server/search.ts \
src/server/routes/search-stream.ts \
src/server/search/engine-cache.ts \
src/server/utils/cache.ts
do
echo "--- $f ---"
ast-grep outline "$f" --items all 2>/dev/null || true
done
echo "=== TARGETED CURRENT IMPLEMENTATIONS ==="
rg -n -C 5 \
-e 'innerHTML' -e 'sanitize' -e 'DOMPurify' \
-e '`#each`' -e '`@index`' \
-e 'parseUrl' -e 'malformed' \
-e 'blocked.*(request|redirect)' -e 'req\.url' -e 'resp\.url' -e 'control' \
-e 'SEARX_FORMAT_PARAM' -e 'format' \
-e 'reloadEngines' -e '_reloadSearx' \
-e 'DEGOOG_PUBLIC_INSTANCE' -e 'DEGOOG_SETTINGS_PASSWORDS' -e 'afterAll' \
-e 'runKey' -e 'readRun' -e 'saveRun' -e 'CachedEngineRun' -e 'engineRunCache' \
src/client/modules/renderer/render-slots.ts \
src/client/utils/template.ts \
src/server/extensions/compatibility-layer/searx/api-shape.ts \
src/server/extensions/compatibility-layer/searx/index.ts \
src/server/routes/search/_search-routes.ts \
src/server/routes/settings.ts \
tests/routes/extensions-upload.test.ts \
src/server/search.ts \
src/server/routes/search-stream.ts \
src/server/search/engine-cache.ts \
src/server/utils/cache.ts
echo "=== FOCUSED TEST COVERAGE ==="
rg -n -C 3 \
-e 'slot.*sanitize|FullWidthAboveResults|renderSlot' \
-e 'template.*each|`@index`|{{0}}' \
-e 'malformed.*URL|CRLF|control.*character' \
-e 'format=json|Searx.*form|reloadEngines' \
-e 'runKey|readRun|invalid.*cache|CachedEngineRun' \
tests --glob '*.{test.ts,test.tsx}' || trueLength of output: 73861 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== PRECISE IMPLEMENTATIONS ==="
for spec in \
"src/client/utils/template.ts:19:42" \
"src/server/routes/settings.ts:221:282" \
"src/server/search.ts:298:410" \
"src/server/routes/search-stream.ts:90:205" \
"src/server/search/engine-cache.ts:36:112" \
"tests/routes/extensions-upload.test.ts:42:115"
do
file="${spec%%:*}"
rest="${spec#*:}"
start="${rest%%:*}"
end="${rest##*:}"
echo "--- $file Lines $start-$end ---"
sed -n "${start},${end}p" "$file" | nl -ba -v "$start"
done
echo "=== FOCUSED TESTS ==="
rg -n -C 4 -F -e 'FullWidthAboveResults' -e '`@index`' -e 'malformed result url' \
-e 'blocked non-http' -e 'format=json' -e 'searxReloadFailed' \
-e 'cache key failed' -e 'cache read failed' -e 'discarded a cached run' \
tests --glob '*.{test.ts,test.tsx}' || true
echo "=== TEST FILES LIKELY TO COVER SEARCH ROUTES, SETTINGS, AND CACHE ==="
fd -t f -e ts . tests | rg '/(search|settings|cache|template|searx).*\.test\.ts$' | sortLength of output: 1432 🏁 Script executed: #!/bin/bash
set -euo pipefail
show() {
local file="$1" start="$2" end="$3"
echo "--- $file Lines $start-$end ---"
sed -n "${start},${end}p" "$file" | awk -v n="$start" '{printf "%d\t%s\n", n++, $0}'
}
echo "=== PRECISE IMPLEMENTATIONS ==="
show src/client/utils/template.ts 19 42
show src/server/routes/settings.ts 221 282
show src/server/search.ts 298 410
show src/server/routes/search-stream.ts 90 205
show src/server/search/engine-cache.ts 36 112
show tests/routes/extensions-upload.test.ts 42 115
echo "=== FOCUSED TEST REFERENCES ==="
rg -n -C 4 -F \
-e 'FullWidthAboveResults' \
-e '`@index`' \
-e 'malformed result url' \
-e 'blocked non-http' \
-e 'format=json' \
-e 'searxReloadFailed' \
-e 'cache key failed' \
-e 'cache read failed' \
-e 'discarded a cached run' \
tests --glob '*.{test.ts,test.tsx}' || true
echo "=== LIKELY TEST FILES ==="
fd -t f -e ts . tests | rg '/(search|settings|cache|template|searx).*\.test\.ts$' | sortLength of output: 21919 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== CACHE AND STREAMING TESTS ==="
rg -n -C 5 \
-e 'readRun' \
-e 'runKey' \
-e 'engineRunCache' \
-e 'invalid.*(cache|run)|cache.*invalid' \
-e 'cache.*(throw|fail|reject)' \
tests/unit tests/routes --glob '*.{test.ts,test.tsx}' || true
echo "=== SEARX BRIDGE URL-LOGGING TESTS ==="
rg -n -C 5 \
-e 'CRLF' \
-e 'control.*(char|url)' \
-e 'blocked non-http' \
-e 'only http' \
-e 'scrubLog' \
tests --glob '*.{test.ts,test.tsx}' || true
echo "=== CURRENT UNRESOLVED CODERABBIT THREAD IDS ==="
gh api graphql \
-F owner="degoog-org" -F name="degoog" -F pr="226" \
-f query='
query($owner: String!, $name: String!, $pr: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $pr) {
reviewThreads(first: 100) {
nodes {
id isResolved path
comments(first: 1) { nodes { author { login } body } }
}
}
}
}
}' --jq '
.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false)
| select(.comments.nodes[0].author.login == "coderabbitai")
| {id,path,body:.comments.nodes[0].body}'Length of output: 25963 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh api graphql \
-F threadId="PRRT_kwDORejnm86VoNsu" \
-f query='
mutation($threadId: ID!) {
resolveReviewThread(input: {threadId: $threadId}) {
thread { id isResolved path }
}
}' \
--jq '.data.resolveReviewThread.thread'Length of output: 489
I confirmed that the eight non-deferred findings are fixed. The changes include focused regression coverage for template array indexes, SearX format fallback, reload-failure reporting, cache failure handling, cache-hit validation, and SearX log control-character removal. I resolved the verified cache-validation thread in Two CodeRabbit threads remain open, as requested:
I did not execute the test suite because the sandbox does not permit repository code execution. You are interacting with an AI system. |
|
Ok so @coderabbitai am I good to merge and release? <3 |
|
Tip For best results, initiate chat on the files or code changes.
The deferred SearX locale keys are not a merge blocker. They can display raw translation keys for non-English users. I also did not run the test suite in the sandbox. After the HTML sanitization fix, run the focused tests and the release CI checks. Then this PR should be ready to merge. 🐇 You are interacting with an AI system. |
@coderabbitai the tests pass just fine. With this in mind is there anything else worrying you or can we merge? |
|
Tip For best results, initiate chat on the files or code changes.
The missing non-English SearX modal locale keys remain a non-blocking UI issue. They can cause raw translation keys for affected users. They do not affect release safety. You can merge and release this PR. ✏️ Learnings added
You are interacting with an AI system. |
|
@coderabbitai let's compromise with you opening an issue for "The missing non-English SearX modal locale keys remain a non-blocking UI issue. They can cause raw translation keys for affected users. They do not affect release safety."? :) I'll release now <3 thank you for all the help buddy |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="degoog-org/degoog"
pr_number="226"
comment_url="$(
gh api --paginate "repos/$repo/issues/$pr_number/comments?per_page=100" \
--jq '.[] | select(.user.login == "fccview" and (.body | contains("let'\''s compromise with you opening an issue"))) | .html_url' \
| tail -n 1
)"
if [[ -z "$comment_url" ]]; then
echo "Could not locate the requester comment URL on PR #$pr_number." >&2
exit 1
fi
issue_body="$(cat <<EOF
## Summary
The SearX engine modal can display raw translation keys for some non-English users.
This does not affect release safety. It is a non-blocking UI localization issue.
## Required changes
Add the locale keys used by \`src/client/settings/engines/searx-modal.ts\` for the SearX update status.
At minimum, add translations for:
- \`settings-page.extensions.searx-updating\`
- \`settings-page.extensions.searx-updated\`
Update the non-English core locale resources and the Degoog theme locale resources that can render this modal.
Affected areas include:
- \`src/client/settings/engines/searx-modal.ts\`
- \`src/locales/fr-FR.json\`
- \`src/locales/he.json\`
- \`src/locales/it.json\`
- Relevant files under \`src/public/themes/degoog-theme/locales/\`
## Rationale
Missing keys can show raw translation-key text instead of localized update status text.
## Acceptance criteria
- The SearX modal resolves both update-status keys for French, Hebrew, and Italian.
- The relevant Degoog theme locale resources contain the required keys.
- The modal does not show raw translation keys for these status messages.
- English behavior remains unchanged.
Requested by `@fccview`.
Backlinks:
- PR `#226`: https://github.com/$repo/pull/$pr_number
- Request comment: $comment_url
EOF
)"
gh issue create \
--repo "$repo" \
--title "Add missing non-English SearX modal update-status locale keys" \
--assignee "fccview" \
--body "$issue_body"Length of output: 2103
The issue links back to PR 🐇✨ You are interacting with an AI system. |
Changelog
Features
above-results-full-widthBugfixes
Interesting behind the scene
For anyone curious to know how the whole searx engines work within degoog here's what I came up with
Degoog is basically leveraging python as if it were running lambdas, which is pretty lightweight, no need to convert anything, having long running processes or acutally running searx. This whole operation adds about
40msoverhead per engine per uncached search, which is a pretty amazing compromise, given the benefits it brings. If you'd rather route searxng via degoog by keeping searxng running as a sidecar instead you can keep the compatibility layer disabled and find the community plugins to do so here clicking on "engines".Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Changes