refactor: Split settings into package and refactor#1570
Merged
Conversation
Contributor
|
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:
WalkthroughThe PR refactors the settings API by splitting a large ChangesSettings Module Restructuring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
backend
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }, | ||
| status_code=409, | ||
| ) | ||
| return JSONResponse({"error": f"Failed to retrieve settings: {str(e)}"}, status_code=500) |
| {"error": f"Failed to update settings: {str(e)}"}, status_code=500 | ||
| Category.SETTINGS_OPERATIONS, MessageId.ORB_SETTINGS_UPDATE_FAILED | ||
| ) | ||
| return JSONResponse({"error": f"Failed to update settings: {str(e)}"}, status_code=500) |
| { | ||
| "error": str(e), | ||
| }, | ||
| {"error": str(e)}, |
| return JSONResponse( | ||
| {"error": f"Failed to rollback onboarding: {str(e)}"}, status_code=500 | ||
| ) | ||
| return JSONResponse({"error": f"Failed to rollback onboarding: {str(e)}"}, status_code=500) |
Collaborator
Author
There was a problem hiding this comment.
@coderabbitai create an issue also add the Prompt for AI Agents in potential fix and link this review by coderabbitai
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh issue create \
--repo langflow-ai/openrag \
--title "Security: Information exposure through exception messages in API error responses" \
--body "## Summary
A CodeQL security scan flagged **CWE-209 (Information Exposure Through an Error Message)** in \`src/api/settings/endpoints.py\`. Exception messages — including internal stack trace details — are forwarded directly to the HTTP response body, which may leak sensitive system internals to external clients.
## Affected Location
**File:** \`src/api/settings/endpoints.py\`
**Function:** \`rollback_onboarding\` (and similar patterns in \`get_settings\`, \`update_settings\`, \`onboarding\`, \`update_onboarding_state\`)
**Example of vulnerable pattern:**
\`\`\`python
except Exception as e:
logger.error(\"Failed to rollback onboarding configuration\", error=str(e))
return JSONResponse({\"error\": f\"Failed to rollback onboarding: {str(e)}\"}, status_code=500)
\`\`\`
The \`str(e)\` interpolation in the JSON response body exposes raw exception details to the caller.
## Risk
- **Severity:** Medium / High
- **Type:** Information Disclosure (CWE-209)
- Internal error messages (file paths, library internals, configuration details, stack traces) can be harvested by a malicious caller to fingerprint the system or tailor further attacks.
## Potential Fix
Replace the raw exception string in all 500-level \`JSONResponse\` returns with a generic client-facing message, while preserving full detail in server-side logs:
\`\`\`python
except Exception as e:
logger.error(\"Failed to rollback onboarding configuration\", error=str(e))
return JSONResponse({\"error\": \"Failed to rollback onboarding. Please try again or contact support.\"}, status_code=500)
\`\`\`
Apply the same pattern consistently across all endpoint exception handlers in the \`src/api/settings/\` package.
### 🤖 Prompt for AI Agents
\`\`\`
In the file src/api/settings/endpoints.py (and the other files in the src/api/settings/ package — helpers.py, langflow_sync.py), audit every `except Exception as e:` block that returns a JSONResponse or raises an HTTPException.
For each occurrence where `str(e)` or `repr(e)` is interpolated directly into the response body (e.g. `{"error": f"...: {str(e)}"}`):
1. Keep the full `str(e)` in the `logger.error(...)` call so internal details are preserved in server logs.
2. Replace the client-facing string with a short, generic human-readable message that does NOT include `str(e)`. Example:
- Before: `return JSONResponse({"error": f"Failed to rollback onboarding: {str(e)}"}, status_code=500)`
- After: `return JSONResponse({"error": "Failed to rollback onboarding. Please try again or contact support."}, status_code=500)`
3. Do the same for any `raise HTTPException(status_code=500, detail=f"...: {str(e)}")` patterns — replace the detail with a generic message.
4. Do NOT change 400-level responses where the error message is already a user-actionable validation message (e.g. "chunk_overlap must be less than chunk_size") — those are intentional.
5. After the changes, run the existing test suite to confirm no regressions.
\`\`\`
## References
- CodeQL finding: [Information exposure through an exception](https://github.com/langflow-ai/openrag/security/code-scanning/118)
- PR introducing the refactor: https://github.com/langflow-ai/openrag/pull/1570
- Flagged review comment: https://github.com/langflow-ai/openrag/pull/1570#discussion_r3212085127
- Requested by: `@edwinjosechittilappilly`
"
Length of output: 2857
🏁 Script executed:
#!/bin/bash
# Search for all occurrences of the vulnerable pattern across the settings package
rg -n 'str\(e\)' src/api/settings/ --type py -A1 -B1
Length of output: 10992
Audit src/api/settings/endpoints.py for every except Exception as e: block that
returns a JSONResponse or raises an HTTPException with status_code >= 500.
For each such occurrence where str(e) or repr(e) appears in the response body or
HTTPException detail:
- Preserve the full
str(e)inside the existinglogger.error(...)call — do NOT
remove it from logs. - Replace the client-facing error string with a concise, generic human-readable message
that does NOT interpolatestr(e). Use the function name as context, e.g.:
"Failed to retrieve settings. Please try again or contact support."
"Failed to update settings. Please try again or contact support."
"Failed to rollback onboarding. Please try again or contact support."
(and so on for each function) - Apply the same fix to
raise HTTPException(status_code=500, detail=f"...: {str(e)}")— replace
the detail string with a generic message. - Do NOT alter 400-level responses where the message is a user-actionable validation
error (e.g. "chunk_overlap must be less than chunk_size", LangflowNotReadyError
503 messages, or provider validation 400s that describe what the user did wrong). - After editing, verify that the existing test suite passes without regressions.
Affected lines (as of PR #1570): 230, 358, 751, 959, 1051, 1096, 1178, 1214, 1433, 1508, 1545.
## References
- CodeQL finding: [Information exposure through an exception — code-scanning `#118`](https://github.com/langflow-ai/openrag/security/code-scanning/118)
- PR introducing the refactor: https://github.com/langflow-ai/openrag/pull/1570
- Flagged review comment by `@coderabbitai`: https://github.com/langflow-ai/openrag/pull/1570#discussion_r3212085127
- Requested by: `@edwinjosechittilappilly`
"""
with tempfile.NamedTemporaryFile(mode='w', suffix='.md', delete=False) as f:
f.write(body)
tmpfile = f.name
result = subprocess.run(
[
'gh', 'issue', 'create',
'--repo', 'langflow-ai/openrag',
'--title', 'Security: Information exposure through exception messages in API error responses',
'--body-file', tmpfile,
'--assignee', 'edwinjosechittilappilly',
],
capture_output=True, text=True
)
os.unlink(tmpfile)
print("STDOUT:", result.stdout)
print("STDERR:", result.stderr)
print("Return code:", result.returncode)
PYEOF
🏁 Script executed:
#!/bin/bash
python3 - <<'PYEOF'
import subprocess, tempfile, os
body = """\
## Summary
A CodeQL security scan flagged **CWE-209 (Information Exposure Through an Error Message)** in `src/api/settings/endpoints.py`. Raw exception messages are interpolated directly into HTTP 500 response bodies via `str(e)`, which may leak internal stack traces, file paths, library internals, or configuration details to external callers.
## Affected Locations
All of the following locations in `src/api/settings/endpoints.py` forward `str(e)` to the client:
| Function | Line(s) | Pattern |
|---|---|---|
| `get_settings` | 230 | `JSONResponse({"error": f"Failed to retrieve settings: {str(e)}"}, 500)` |
| `update_settings` | 358 | `JSONResponse({"error": f"{str(e)}"}, 400)` *(provider validation — intentional, but leaks provider error detail)* |
| `update_settings` | 751 | `JSONResponse({"error": f"Failed to update settings: {str(e)}"}, 500)` |
| `onboarding` | 959 | `JSONResponse({"error": str(e)}, 400)` |
| `onboarding` | 1051 | `JSONResponse({"error": str(e)}, 500)` |
| `onboarding` | 1096 | `JSONResponse({"error": f"Failed to ingest sample documents: {str(e)}"}, 500)` |
| `onboarding` | 1178 | `JSONResponse({"error": str(e)}, 500)` |
| `update_onboarding_state` | 1214 | `JSONResponse({"error": f"Failed to update onboarding state: {str(e)}"}, 500)` |
| `rollback_onboarding` | **1433** *(CodeQL flagged)* | `JSONResponse({"error": f"Failed to rollback onboarding: {str(e)}"}, 500)` |
| `update_docling_preset` | 1508 | `HTTPException(500, detail=f"Failed to update docling settings: {str(e)}")` |
| `refresh_openrag_docs` | 1545 | `HTTPException(500, detail=f"Failed to refresh OpenRAG docs: {str(e)}")` |
## Risk
- **Severity:** Medium
- **Type:** Information Disclosure (CWE-209)
- Raw exception strings can expose internal paths, library version details, configuration values, or credentials fragments, which can be used by an attacker to fingerprint the system or craft more targeted attacks.
## Potential Fix
For every 500-level response that interpolates `str(e)`, keep the full detail in the server-side log and replace the client-facing string with a short, generic message:
```python
# Before
except Exception as e:
logger.error("Failed to rollback onboarding configuration", error=str(e))
return JSONResponse({"error": f"Failed to rollback onboarding: {str(e)}"}, status_code=500)
# After
except Exception as e:
logger.error("Failed to rollback onboarding configuration", error=str(e))
return JSONResponse({"error": "Failed to rollback onboarding. Please try again or contact support."}, status_code=500)
```
Apply consistently across all affected handlers listed above.
### 🤖 Prompt for AI Agents
Length of output: 226
🏁 Script executed:
#!/bin/bash
cat > /tmp/create_issue.py << 'ENDOFPYTHON'
import subprocess, os
body = (
"## Summary\n\n"
"A CodeQL security scan flagged **CWE-209 (Information Exposure Through an Error Message)** in "
"`src/api/settings/endpoints.py`. Raw exception messages are interpolated directly into HTTP 500 "
"response bodies via `str(e)`, which may leak internal stack traces, file paths, library internals, "
"or configuration details to external callers.\n\n"
"## Affected Locations\n\n"
"All of the following locations in `src/api/settings/endpoints.py` forward `str(e)` to the client:\n\n"
"| Function | Line(s) | Status |\n"
"|---|---|---|\n"
"| `get_settings` | 230 | 500 |\n"
"| `update_settings` (provider validation) | 358 | 400 |\n"
"| `update_settings` (outer handler) | 751 | 500 |\n"
"| `onboarding` (provider validation) | 959 | 400 |\n"
"| `onboarding` (OpenSearch init) | 1051 | 500 |\n"
"| `onboarding` (sample data ingestion) | 1096 | 500 |\n"
"| `onboarding` (outer handler) | 1178 | 500 |\n"
"| `update_onboarding_state` | 1214 | 500 |\n"
"| `rollback_onboarding` **(CodeQL flagged)** | 1433 | 500 |\n"
"| `update_docling_preset` | 1508 | 500 |\n"
"| `refresh_openrag_docs` | 1545 | 500 |\n\n"
"## Risk\n\n"
"- **Severity:** Medium\n"
"- **Type:** Information Disclosure (CWE-209)\n"
"- Raw exception strings can expose internal paths, library version details, configuration values, "
"or credential fragments, which can help an attacker fingerprint the system or craft more targeted attacks.\n\n"
"## Potential Fix\n\n"
"For every 500-level response that interpolates `str(e)`, keep the full detail in the server-side "
"log and replace the client-facing string with a short, generic message:\n\n"
"```python\n"
"# Before\n"
"except Exception as e:\n"
' logger.error("Failed to rollback onboarding configuration", error=str(e))\n'
' return JSONResponse({"error": f"Failed to rollback onboarding: {str(e)}"}, status_code=500)\n\n'
"# After\n"
"except Exception as e:\n"
' logger.error("Failed to rollback onboarding configuration", error=str(e))\n'
' return JSONResponse({"error": "Failed to rollback onboarding. Please try again or contact support."}, status_code=500)\n'
"```\n\n"
"Apply consistently across all affected handlers listed in the table above.\n\n"
"### Prompt for AI Agents\n\n"
"Audit `src/api/settings/endpoints.py` for every `except Exception as e:` block that "
"returns a JSONResponse or raises an HTTPException with status_code >= 500.\n\n"
"For each occurrence where `str(e)` appears in the response body or HTTPException detail:\n\n"
"1. Preserve the full `str(e)` inside the existing `logger.error(...)` call — do NOT remove it from logs.\n"
"2. Replace the client-facing error string with a concise, generic human-readable message "
"that does NOT interpolate `str(e)`. Use the function name as context, for example:\n"
' - `"Failed to retrieve settings. Please try again or contact support."`\n'
' - `"Failed to update settings. Please try again or contact support."`\n'
' - `"Failed to rollback onboarding. Please try again or contact support."`\n\n'
"3. Apply the same fix to `raise HTTPException(status_code=500, detail=f'...: {str(e)}')` — "
"replace the detail string with a generic message.\n"
"4. Do NOT alter 400-level responses where the message is a user-actionable validation error "
"(e.g. 'chunk_overlap must be less than chunk_size', LangflowNotReadyError 503 messages, or "
"provider validation 400s that describe what the user did wrong).\n"
"5. After editing, verify that the existing test suite passes without regressions.\n\n"
"Affected lines (as of PR `#1570`): 230, 358, 751, 959, 1051, 1096, 1178, 1214, 1433, 1508, 1545.\n\n"
"## References\n\n"
"- CodeQL finding: [Information exposure through an exception — code-scanning `#118`]"
"(https://github.com/langflow-ai/openrag/security/code-scanning/118)\n"
"- PR introducing the refactor: https://github.com/langflow-ai/openrag/pull/1570\n"
"- Flagged review comment by `@coderabbitai`: https://github.com/langflow-ai/openrag/pull/1570#discussion_r3212085127\n"
"- Requested by: `@edwinjosechittilappilly`\n"
)
with open("/tmp/issue_body.md", "w") as f:
f.write(body)
result = subprocess.run(
[
"gh", "issue", "create",
"--repo", "langflow-ai/openrag",
"--title", "Security: Information exposure through exception messages in API error responses",
"--body-file", "/tmp/issue_body.md",
"--assignee", "edwinjosechittilappilly",
],
capture_output=True, text=True
)
print("STDOUT:", result.stdout)
print("STDERR:", result.stderr)
print("Return code:", result.returncode)
ENDOFPYTHON
python3 /tmp/create_issue.py
Length of output: 84
@edwinjosechittilappilly, the GitHub issue has been created and assigned to you:
➡️ #1589
The issue includes:
- Summary — CWE-209 information disclosure via
str(e)in response bodies - Affected Locations table — all 11 occurrences found across
rollback_onboarding,get_settings,update_settings,onboarding,update_onboarding_state,update_docling_preset, andrefresh_openrag_docs - Before/after code fix showing the pattern to apply
- 🤖 Prompt for AI Agents — a ready-to-use prompt that instructs an AI agent to:
- Keep
str(e)inlogger.error()calls (server-side logs only) - Replace client-facing 500-level error strings with generic messages
- Fix
HTTPException(detail=f"...: {str(e)}")patterns too - Leave intentional 400-level user-facing validation messages untouched
- Verify the test suite after changes
- Keep
- References — links to the CodeQL finding (
#118), PR#1570, and this review comment
🧠 Learnings used
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/openrag PR: 1538
File: src/api/config.py:21-33
Timestamp: 2026-05-07T13:39:12.243Z
Learning: In the langflow-ai/openrag repo, treat `src/api/v1/` as reserved exclusively for SDK-supported public APIs. Do not place non-SDK public endpoints there (for example, pre-auth frontend-only endpoints like `/onboarding-status`). Non-SDK public endpoints should be implemented directly under `src/api/` instead. When reviewing, do not flag non-SDK endpoint code that lives outside `src/api/v1/` as a violation.
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the settings/onboarding API implementation by splitting the former monolithic api.settings module into a proper package with dedicated submodules for endpoints, Pydantic models, shared helpers, and Langflow synchronization—while preserving the original import surface via api/settings/__init__.py.
Changes:
- Introduces
api.settingsas a package withendpoints.py,models.py,helpers.py, andlangflow_sync.py, and re-exports prior public symbols from__init__.py. - Updates lint/type-check configuration to better accommodate FastAPI dependency injection patterns (Ruff/Bugbear) and known mypy return-type constraints in the settings endpoints module.
- Minor logging/formatting tweaks in
src/api/router.py(including traceback logging).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/api/settings/models.py | New module containing the Pydantic request/response models previously defined in the flat settings module. |
| src/api/settings/langflow_sync.py | New module holding Langflow synchronization logic, including re-sync entrypoint used by startup orchestration. |
| src/api/settings/helpers.py | New module for shared helper functions (provider fallback selection, embedding conflict detection, OpenRAG docs filter creation). |
| src/api/settings/endpoints.py | Consolidates the FastAPI route handlers after the split; imports helpers/models/sync helpers from their new modules. |
| src/api/settings/init.py | Re-exports endpoints + models (and reapply_all_settings) to preserve the original api.settings public import contract. |
| src/api/router.py | Minor spacing plus explicit full traceback logging on task-based Langflow upload failures. |
| pyproject.toml | Ruff flake8-bugbear configuration for FastAPI-style defaults and a scoped mypy override for api.settings.endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/router.py (1)
12-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix import ordering to unblock CI (Ruff I001).
The
dependenciesimport members are not sorted, which is triggering the lint failure reported at Line 3 and blocking the pipeline.Proposed fix
from dependencies import ( + get_current_user, get_document_service, get_langflow_file_service, get_session_manager, get_task_service, - get_current_user, )🤖 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/api/router.py` around lines 12 - 18, The grouped import from dependencies is not alphabetically ordered and triggers Ruff I001; reorder the imported members get_current_user, get_document_service, get_langflow_file_service, get_session_manager, and get_task_service into alphabetical order inside the import tuple (or otherwise sort them consistently) so the names are sorted and the linter passes.
🧹 Nitpick comments (4)
src/api/settings/endpoints.py (2)
1031-1053: ⚖️ Poor tradeoffOpenSearch init failure leaves partially-applied config un-persisted.
Onboarding mutates
current_configextensively (provider keys, models, configured flags) before reachinginit_indexat line 1043. Wheninit_indexraises, the handler returns 500 (line 1050) without persisting viaconfig_manager.save_config_file(...)—save_config_fileis only called later at line 1100. As a result, the user-supplied API keys, endpoints, and provider state are silently dropped on this error path, and the next onboarding attempt has to re-collect them.This is pre-existing behavior lifted verbatim, so flagging only as a refactor — but worth tracking, since the post-validation, post-Langflow-sync state would more usefully be persisted (or explicitly rolled back) before returning.
🤖 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/api/settings/endpoints.py` around lines 1031 - 1053, The onboarding handler mutates current_config then calls init_index (init_index) and on failure returns 500 without persisting those changes; ensure you persist or rollback the partially-applied configuration by calling config_manager.save_config_file(current_config) immediately after mutations (before awaiting init_index) or, alternatively, capture the exception from init_index and either save the mutated current_config in the exception path or restore the prior config and then save; identify the mutation site around current_config modifications in this handler and add a save_config_file call (or explicit rollback logic) so provider keys/models/configured flags are not lost when init_index fails.
230-230: ⚡ Quick winAvoid leaking exception messages to API responses (CodeQL).
CodeQL flags four error paths in this file (lines 230, 751, 1051, 1428) where the raw
str(e)is interpolated into the JSON error body. While these are mostly verbatim lifts, the surrounding lines were touched in this refactor. Consider returning a generic error message to clients while keeping the detailed exception in server-side logs (which already happens vialogger.error(...)). The same applies to theHTTPException(detail=f"... {str(e)}")calls on lines 1503 and 1540–1541.♻️ Example fix pattern
- return JSONResponse({"error": f"Failed to retrieve settings: {str(e)}"}, status_code=500) + return JSONResponse({"error": "Failed to retrieve settings"}, status_code=500)Also applies to: 751-751, 1051-1051, 1428-1428
🤖 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/api/settings/endpoints.py` at line 230, Several API handlers in settings/endpoints.py are returning exception text directly to clients (e.g., the JSONResponse call that uses f"Failed to retrieve settings: {str(e)}" and the HTTPException(detail=...) occurrences); change these to return a generic client-facing error message (e.g., "Failed to retrieve settings" or "Internal server error") while preserving the detailed exception only in server logs via existing logger.error(...) calls; update all occurrences referenced (the JSONResponse returning f"...{str(e)}" and the HTTPException(detail=f"...{str(e)}") usages) so no raw exception text is interpolated into responses but is still logged for debugging.pyproject.toml (1)
143-151: ⚡ Quick winModule-wide
return-valuesuppression hides future type bugs.Disabling
return-valuefor the entireapi.settings.endpointsmodule silences real return-type mismatches that have nothing to do with the documentedJSONResponseworkaround. Consider one of:
- Per-line
# type: ignore[return-value]on the specificreturn JSONResponse(...)lines, or- Annotate handlers as returning
Response(FastAPI still usesresponse_model=...for OpenAPI when set on the route decorator), avoiding the union/return-valueissue entirely.🤖 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 `@pyproject.toml` around lines 143 - 151, Remove the module-wide mypy return-value suppression for api.settings.endpoints and instead either add per-line ignores on the specific lines that return JSONResponse (use "# type: ignore[return-value]" on those return JSONResponse(...) statements) or change the FastAPI handler return annotations to starlette.responses.Response (or Response from fastapi) so the annotated return type covers both the pydantic model and JSONResponse while keeping response_model=... on the route; update the override block in pyproject.toml accordingly to avoid disabling "return-value" for the whole module.src/api/settings/helpers.py (1)
173-174: ⚡ Quick winReplace deprecated
datetime.utcnow()withdatetime.now(timezone.utc).The project targets Python 3.13, where
datetime.utcnow()is deprecated and emitsDeprecationWarning. The recommended replacement is timezone-aware.♻️ Suggested change
-from datetime import datetime +from datetime import datetime, timezone @@ - "created_at": datetime.utcnow().isoformat(), - "updated_at": datetime.utcnow().isoformat(), + "created_at": datetime.now(timezone.utc).isoformat(), + "updated_at": datetime.now(timezone.utc).isoformat(),🤖 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/api/settings/helpers.py` around lines 173 - 174, Replace the deprecated datetime.utcnow() calls used for the "created_at" and "updated_at" fields with timezone-aware calls: use datetime.now(timezone.utc).isoformat() instead of datetime.utcnow().isoformat(), and add the necessary import for timezone from datetime (or switch the datetime import to `from datetime import datetime, timezone`) so the assignments for "created_at" and "updated_at" become timezone-aware.
🤖 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.
Outside diff comments:
In `@src/api/router.py`:
- Around line 12-18: The grouped import from dependencies is not alphabetically
ordered and triggers Ruff I001; reorder the imported members get_current_user,
get_document_service, get_langflow_file_service, get_session_manager, and
get_task_service into alphabetical order inside the import tuple (or otherwise
sort them consistently) so the names are sorted and the linter passes.
---
Nitpick comments:
In `@pyproject.toml`:
- Around line 143-151: Remove the module-wide mypy return-value suppression for
api.settings.endpoints and instead either add per-line ignores on the specific
lines that return JSONResponse (use "# type: ignore[return-value]" on those
return JSONResponse(...) statements) or change the FastAPI handler return
annotations to starlette.responses.Response (or Response from fastapi) so the
annotated return type covers both the pydantic model and JSONResponse while
keeping response_model=... on the route; update the override block in
pyproject.toml accordingly to avoid disabling "return-value" for the whole
module.
In `@src/api/settings/endpoints.py`:
- Around line 1031-1053: The onboarding handler mutates current_config then
calls init_index (init_index) and on failure returns 500 without persisting
those changes; ensure you persist or rollback the partially-applied
configuration by calling config_manager.save_config_file(current_config)
immediately after mutations (before awaiting init_index) or, alternatively,
capture the exception from init_index and either save the mutated current_config
in the exception path or restore the prior config and then save; identify the
mutation site around current_config modifications in this handler and add a
save_config_file call (or explicit rollback logic) so provider
keys/models/configured flags are not lost when init_index fails.
- Line 230: Several API handlers in settings/endpoints.py are returning
exception text directly to clients (e.g., the JSONResponse call that uses
f"Failed to retrieve settings: {str(e)}" and the HTTPException(detail=...)
occurrences); change these to return a generic client-facing error message
(e.g., "Failed to retrieve settings" or "Internal server error") while
preserving the detailed exception only in server logs via existing
logger.error(...) calls; update all occurrences referenced (the JSONResponse
returning f"...{str(e)}" and the HTTPException(detail=f"...{str(e)}") usages) so
no raw exception text is interpolated into responses but is still logged for
debugging.
In `@src/api/settings/helpers.py`:
- Around line 173-174: Replace the deprecated datetime.utcnow() calls used for
the "created_at" and "updated_at" fields with timezone-aware calls: use
datetime.now(timezone.utc).isoformat() instead of datetime.utcnow().isoformat(),
and add the necessary import for timezone from datetime (or switch the datetime
import to `from datetime import datetime, timezone`) so the assignments for
"created_at" and "updated_at" become timezone-aware.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20a9abd2-0b81-43f6-a0cb-6695114a1a8f
📒 Files selected for processing (7)
pyproject.tomlsrc/api/router.pysrc/api/settings/__init__.pysrc/api/settings/endpoints.pysrc/api/settings/helpers.pysrc/api/settings/langflow_sync.pysrc/api/settings/models.py
edwinjosechittilappilly
force-pushed
the
refactor-api-settings
branch
2 times, most recently
from
May 11, 2026 13:46
e329b50 to
c79d73d
Compare
github-actions
Bot
added
refactor
ci
Contributor
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 (2)
src/api/settings/endpoints.py (2)
1100-1131:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist
edited=Truebefore reporting onboarding success.
save_config_file(current_config)runs whilecurrent_config.editedis stillFalse. Unless a later save happens incidentally, the onboarding-complete flag only exists in memory and is lost on restart.Suggested fix
- if config_manager.save_config_file(current_config): + current_config.edited = True + if config_manager.save_config_file(current_config): set_fields = [k for k, v in body.model_dump(exclude_unset=True).items()] logger.info( "Onboarding configuration updated successfully", updated_fields=set_fields, ) - # Mark config as edited and send telemetry with model information - current_config.edited = True - # Build metadata with selected models onboarding_metadata = {}🤖 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/api/settings/endpoints.py` around lines 1100 - 1131, The current flow calls config_manager.save_config_file(current_config) while current_config.edited is still False so the edited flag isn't persisted; update the sequence in the endpoint so you set current_config.edited = True before persisting (i.e., assign edited=True on the current_config object prior to calling config_manager.save_config_file) and then call config_manager.save_config_file(current_config) so the edited flag is saved; keep the telemetry sends via TelemetryClient.send_event (Category.ONBOARDING, MessageId.ORB_ONBOARD_CONFIG_EDITED/ORB_ONBOARD_COMPLETE) after the persisted save.
766-1167: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftMove the onboarding orchestration out of the route handler.
This handler still owns config mutation, provider validation, Langflow sync, OpenSearch init, sample ingestion, persistence, and telemetry. That keeps the API layer tightly coupled to business rules and makes failure/rollback paths much harder to test safely.
As per coding guidelines,
src/api/**/*.py: “No business logic in route handlers.”🤖 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/api/settings/endpoints.py` around lines 766 - 1167, The route handler contains heavy onboarding orchestration (config mutation via get_openrag_config/current_config, provider validation via validate_provider_setup, Langflow sync via wait_for_langflow/_update_langflow_global_variables/_update_mcp_server_urls/_update_langflow_model_values, OpenSearch init via init_index, sample ingestion via ingest_default_documents_when_ready, filter creation via _create_openrag_docs_filter, persistence via config_manager.save_config_file, and telemetry TelemetryClient.send_event) and must be moved into a dedicated service. Create a new onboarding service function (e.g., orchestrate_onboarding(current_user, body, services...)) that encapsulates: applying body to current_config, marking providers configured, full provider validation, waiting for Langflow and updating globals/models, initializing OpenSearch, handling sample data ingestion and filter creation, saving config, sending telemetry, and returning a structured result (success, edited, openrag_docs_filter_id, task_id, error info). Replace the inline logic in the route with a single call to this service, handle its returned result to produce the HTTP response, and ensure all thrown exceptions are normalized to service errors so the route only maps them to appropriate HTTP status codes; keep helper calls (get_openrag_config, validate_provider_setup, wait_for_langflow, _update_langflow_global_variables/_update_mcp_server_urls/_update_langflow_model_values, init_index, ingest_default_documents_when_ready, _create_openrag_docs_filter, config_manager.save_config_file, TelemetryClient.send_event, clients.refresh_patched_client) inside the new service.
🤖 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/api/settings/endpoints.py`:
- Around line 1500-1504: The broad except block is converting intended
HTTPException(400) into a 500; update the error handling in the endpoint in
src/api/settings/endpoints.py so that if the caught exception is an
HTTPException it is immediately re-raised (e.g. check isinstance(e,
HTTPException) and raise), otherwise log the error with logger.error and raise a
new HTTPException(status_code=500, ...) from e; keep references to logger and
HTTPException in the same handler so the original 400 responses for invalid
docling presets are preserved.
In `@src/api/settings/langflow_sync.py`:
- Around line 193-216: The fallback branch in reapply_all_settings() only
updates embedding providers and never forces LLM updates; modify this block in
src/api/settings/langflow_sync.py (inside reapply_all_settings) to also iterate
all configured LLM providers and call flows_service.change_langflow_model_value
for each, passing llm_model = config.knowledge.llm_model when provider ==
current_llm_provider (else None) and setting force_llm_update=True (analogous to
the embedding loop); keep the existing embedding update loop and add a similar
llm_providers collection (e.g., check config.providers.openai.configured,
watsonx, ollama) and a current_llm_provider =
config.knowledge.llm_provider.lower() to decide which provider gets the
configured model.
---
Outside diff comments:
In `@src/api/settings/endpoints.py`:
- Around line 1100-1131: The current flow calls
config_manager.save_config_file(current_config) while current_config.edited is
still False so the edited flag isn't persisted; update the sequence in the
endpoint so you set current_config.edited = True before persisting (i.e., assign
edited=True on the current_config object prior to calling
config_manager.save_config_file) and then call
config_manager.save_config_file(current_config) so the edited flag is saved;
keep the telemetry sends via TelemetryClient.send_event (Category.ONBOARDING,
MessageId.ORB_ONBOARD_CONFIG_EDITED/ORB_ONBOARD_COMPLETE) after the persisted
save.
- Around line 766-1167: The route handler contains heavy onboarding
orchestration (config mutation via get_openrag_config/current_config, provider
validation via validate_provider_setup, Langflow sync via
wait_for_langflow/_update_langflow_global_variables/_update_mcp_server_urls/_update_langflow_model_values,
OpenSearch init via init_index, sample ingestion via
ingest_default_documents_when_ready, filter creation via
_create_openrag_docs_filter, persistence via config_manager.save_config_file,
and telemetry TelemetryClient.send_event) and must be moved into a dedicated
service. Create a new onboarding service function (e.g.,
orchestrate_onboarding(current_user, body, services...)) that encapsulates:
applying body to current_config, marking providers configured, full provider
validation, waiting for Langflow and updating globals/models, initializing
OpenSearch, handling sample data ingestion and filter creation, saving config,
sending telemetry, and returning a structured result (success, edited,
openrag_docs_filter_id, task_id, error info). Replace the inline logic in the
route with a single call to this service, handle its returned result to produce
the HTTP response, and ensure all thrown exceptions are normalized to service
errors so the route only maps them to appropriate HTTP status codes; keep helper
calls (get_openrag_config, validate_provider_setup, wait_for_langflow,
_update_langflow_global_variables/_update_mcp_server_urls/_update_langflow_model_values,
init_index, ingest_default_documents_when_ready, _create_openrag_docs_filter,
config_manager.save_config_file, TelemetryClient.send_event,
clients.refresh_patched_client) inside the new service.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2612667a-b712-4e66-a6c1-b7e23e1e3b26
📒 Files selected for processing (7)
pyproject.tomlsrc/api/router.pysrc/api/settings/__init__.pysrc/api/settings/endpoints.pysrc/api/settings/helpers.pysrc/api/settings/langflow_sync.pysrc/api/settings/models.py
✅ Files skipped from review due to trivial changes (2)
- src/api/settings/init.py
- src/api/router.py
🚧 Files skipped from review as they are similar to previous changes (3)
- src/api/settings/helpers.py
- src/api/settings/models.py
- pyproject.toml
Comment on lines
1500
to
+1504
| except Exception as e: | ||
| logger.error("Failed to update docling settings", error=str(e)) | ||
| raise HTTPException(status_code=500, detail=f"Failed to update docling settings: {str(e)}") | ||
| raise HTTPException( | ||
| status_code=500, detail=f"Failed to update docling settings: {str(e)}" | ||
| ) from e |
Contributor
There was a problem hiding this comment.
Preserve the 400 for invalid docling presets.
This except Exception also catches the HTTPException(status_code=400) raised above for invalid presets, so clients get a 500 instead of the intended 400.
Suggested fix
- except Exception as e:
+ except HTTPException:
+ raise
+ except Exception as e:
logger.error("Failed to update docling settings", error=str(e))
raise HTTPException(
- status_code=500, detail=f"Failed to update docling settings: {str(e)}"
+ status_code=500, detail="Failed to update docling settings"
) from eAs per coding guidelines, src/api/**/*.py: “Verify ... correct HTTP status codes.”
🤖 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/api/settings/endpoints.py` around lines 1500 - 1504, The broad except
block is converting intended HTTPException(400) into a 500; update the error
handling in the endpoint in src/api/settings/endpoints.py so that if the caught
exception is an HTTPException it is immediately re-raised (e.g. check
isinstance(e, HTTPException) and raise), otherwise log the error with
logger.error and raise a new HTTPException(status_code=500, ...) from e; keep
references to logger and HTTPException in the same handler so the original 400
responses for invalid docling presets are preserved.
Collaborator
Author
There was a problem hiding this comment.
@coderabbitai create an issue also add the Prompt for AI Agents in potential fix and link this review by coderabbitai
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh issue create \
--repo langflow-ai/openrag \
--title "bug: HTTPException(400) swallowed as 500 in update_docling_preset" \
--body "## Bug Report
### Description
In \`src/api/settings/endpoints.py\`, the \`update_docling_preset\` endpoint raises an \`HTTPException(status_code=400)\` for invalid preset values, but the broad \`except Exception\` block at the end of the function catches it and re-raises it as an \`HTTPException(status_code=500)\`. This means clients receive a 500 Internal Server Error instead of the intended 400 Bad Request.
### Affected Code
**File:** \`src/api/settings/endpoints.py\` (~lines 1500–1509)
\`\`\`python
except Exception as e:
logger.error(\"Failed to update docling settings\", error=str(e))
raise HTTPException(
status_code=500, detail=f\"Failed to update docling settings: {str(e)}\"
) from e
\`\`\`
### Proposed Fix
Re-raise \`HTTPException\` immediately before the generic handler so the intended 400 status codes are preserved:
\`\`\`diff
- except Exception as e:
+ except HTTPException:
+ raise
+ except Exception as e:
logger.error(\"Failed to update docling settings\", error=str(e))
raise HTTPException(
- status_code=500, detail=f\"Failed to update docling settings: {str(e)}\"
+ status_code=500, detail=\"Failed to update docling settings\"
) from e
\`\`\`
### 🤖 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/api/settings/endpoints.py\` around lines 1500 - 1504, The broad except
block is converting intended HTTPException(400) into a 500; update the error
handling in the endpoint in src/api/settings/endpoints.py so that if the caught
exception is an HTTPException it is immediately re-raised (e.g. check
isinstance(e, HTTPException) and raise), otherwise log the error with
logger.error and raise a new HTTPException(status_code=500, ...) from e; keep
references to logger and HTTPException in the same handler so the original 400
responses for invalid docling presets are preserved.
\`\`\`
### References
- **PR:** langflow-ai/openrag#1570
- **Review comment:** https://github.com/langflow-ai/openrag/pull/1570#discussion_r3219472252
- **Reported by:** `@edwinjosechittilappilly`"
Length of output: 51
@edwinjosechittilappilly, the issue has been created: bug: HTTPException(400) swallowed as 500 in update_docling_preset
It includes:
- A description of the bug and the affected code in
src/api/settings/endpoints.py - The proposed fix diff
- The 🤖 Prompt for AI Agents
- A backlink to PR
#1570and this review comment
🧬 Code Graph Analysis Results
-
File: src/api/settings/endpoints.py
- Summary: System prompt handling within update_settings (summary)
- Description:
In update_settings, when the request body includes a new system_prompt, the code:- updates current_config.agent.system_prompt to body.system_prompt
- marks the config as updated and emits a telemetry event (ORB_SETTINGS_SYSTEM_PROMPT)
- attempts to propagate the new system prompt to Langflow by calling _update_langflow_system_prompt(current_config, flows_service)
- if updating Langflow fails, logs the error but does not fail the entire settings update (continues saving the updated config)
- Rationale: Facilitates prompt customization for AI Agents in a centralized settings workflow and ensures Langflow stays in sync where possible.
-
File: src/api/settings/endpoints.py
- Exact snippet: SettingsUpdateBody class (for context on system_prompt handling)
- Line range: 15-40
- Snippet:
class SettingsUpdateBody(BaseModel):
llm_model: str | None = Field(None, min_length=1)
llm_provider: str | None = Field(None, pattern="^(openai|anthropic|watsonx|ollama)$")
system_prompt: str | None = None
chunk_size: int | None = Field(None, gt=0)
chunk_overlap: int | None = Field(None, ge=0)
table_structure: bool | None = None
ocr: bool | None = None
picture_descriptions: bool | None = None
embedding_model: str | None = Field(None, min_length=1)
embedding_provider: str | None = Field(None, pattern="^(openai|watsonx|ollama)$")
index_name: str | None = Field(None, min_length=1)
openai_api_key: str | None = Field(None, min_length=1)
anthropic_api_key: str | None = Field(None, min_length=1)
watsonx_api_key: str | None = Field(None, min_length=1)
watsonx_endpoint: str | None = Field(None, min_length=1)
watsonx_project_id: str | None = Field(None, min_length=1)
ollama_endpoint: str | None = Field(None, min_length=1)
remove_ollama_config: bool | None = None
remove_openai_config: bool | None = None
remove_anthropic_config: bool | None = None
remove_watsonx_config: bool | None = None
# Explicit confirmation that the caller accepts removing a provider whose
# embedding models are still in use by indexed documents. Without this,
# the backend returns 409 and the frontend prompts the user.
force_remove: bool | None = False
-
File: src/api/settings/models.py
- Snippet: AgentConfig class (context for system_prompt field)
- Line range: 133-136
- Snippet:
class AgentConfig(BaseModel):
llm_model: str | None
llm_provider: str | None
system_prompt: str | None
-
File: src/api/settings/endpoints.py
- Snippet: System prompt update interaction (for context around Langflow sync)
- Line range: (summary only)
- Snippet (summary description):
When body.system_prompt is not None in update_settings:- set current_config.agent.system_prompt = body.system_prompt
- set config_updated = True
- log telemetry ORB_SETTINGS_SYSTEM_PROMPT
- attempt to propagate to Langflow via _update_langflow_system_prompt(current_config, flows_service)
- if Langflow update fails, log error but do not abort the overall settings update
-
File: src/api/settings/endpoints.py
- Snippet: Langflow update call (for context)
- Line range: (summary only)
- Snippet (summary description):
The code wraps _update_langflow_system_prompt(current_config, flows_service) in a try/except; on exception it logs and continues without failing the entire update.
🧠 Learnings used
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/openrag PR: 1538
File: src/api/config.py:21-33
Timestamp: 2026-05-07T13:39:12.243Z
Learning: In the langflow-ai/openrag repo, treat `src/api/v1/` as reserved exclusively for SDK-supported public APIs. Do not place non-SDK public endpoints there (for example, pre-auth frontend-only endpoints like `/onboarding-status`). Non-SDK public endpoints should be implemented directly under `src/api/` instead. When reviewing, do not flag non-SDK endpoint code that lives outside `src/api/v1/` as a violation.
Comment on lines
+193
to
+216
| if not (embedding_model or embedding_provider or llm_model or llm_provider): | ||
| # 2. Update ALL configured embedding providers | ||
| embedding_providers = [] | ||
| if config.providers.openai.configured: | ||
| embedding_providers.append("openai") | ||
| if config.providers.watsonx.configured: | ||
| embedding_providers.append("watsonx") | ||
| if config.providers.ollama.configured: | ||
| embedding_providers.append("ollama") | ||
|
|
||
| current_embedding_provider = config.knowledge.embedding_provider.lower() | ||
| for provider in embedding_providers: | ||
| # Use configured model for current provider, or None (first available) for others | ||
| embedding_model = ( | ||
| config.knowledge.embedding_model | ||
| if provider == current_embedding_provider | ||
| else None | ||
| ) | ||
| await flows_service.change_langflow_model_value( | ||
| provider, embedding_model=embedding_model, force_embedding_update=True | ||
| ) | ||
| logger.info( | ||
| f"Successfully updated Langflow flows for embedding provider {provider}" | ||
| ) |
Contributor
There was a problem hiding this comment.
Reapply-all currently skips the active LLM sync.
When reapply_all_settings() reaches this fallback branch with no explicit overrides, it only refreshes embedding providers. The chat flow never gets a force_llm_update, so a flow reset can come back with a stale/default LLM selection.
Suggested fix
if not (embedding_model or embedding_provider or llm_model or llm_provider):
+ await flows_service.change_langflow_model_value(
+ config.agent.llm_provider.lower(),
+ llm_model=config.agent.llm_model,
+ force_llm_update=True,
+ )
+ logger.info(
+ f"Successfully updated Langflow flows for LLM provider {config.agent.llm_provider.lower()}"
+ )
+
# 2. Update ALL configured embedding providers
embedding_providers = []🤖 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/api/settings/langflow_sync.py` around lines 193 - 216, The fallback
branch in reapply_all_settings() only updates embedding providers and never
forces LLM updates; modify this block in src/api/settings/langflow_sync.py
(inside reapply_all_settings) to also iterate all configured LLM providers and
call flows_service.change_langflow_model_value for each, passing llm_model =
config.knowledge.llm_model when provider == current_llm_provider (else None) and
setting force_llm_update=True (analogous to the embedding loop); keep the
existing embedding update loop and add a similar llm_providers collection (e.g.,
check config.providers.openai.configured, watsonx, ollama) and a
current_llm_provider = config.knowledge.llm_provider.lower() to decide which
provider gets the configured model.
Collaborator
Author
There was a problem hiding this comment.
@coderabbitai create an issue also add the Prompt for AI Agents in potential fix and link this review by coderabbitai
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
gh issue create \
--repo langflow-ai/openrag \
--title "bug: reapply_all_settings() skips LLM sync in fallback branch" \
--body "## Summary
In \`src/api/settings/langflow_sync.py\`, the fallback branch of \`_update_langflow_model_values\` (reached when \`reapply_all_settings()\` is called with no explicit overrides) only refreshes embedding providers. The chat flow never receives a \`force_llm_update\`, so after a flow reset the LLM selection can come back with a stale or default model.
**File:** \`src/api/settings/langflow_sync.py\` (lines 193–216)
## Suggested Fix
\`\`\`diff
if not (embedding_model or embedding_provider or llm_model or llm_provider):
+ await flows_service.change_langflow_model_value(
+ config.agent.llm_provider.lower(),
+ llm_model=config.agent.llm_model,
+ force_llm_update=True,
+ )
+ logger.info(
+ f\"Successfully updated Langflow flows for LLM provider {config.agent.llm_provider.lower()}\"
+ )
+
# 2. Update ALL configured embedding providers
embedding_providers = []
\`\`\`
## 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/api/settings/langflow_sync.py\` around lines 193–216, the fallback branch in \`reapply_all_settings()\` only updates embedding providers and never forces LLM updates; modify this block in \`src/api/settings/langflow_sync.py\` (inside \`reapply_all_settings\`) to also iterate all configured LLM providers and call \`flows_service.change_langflow_model_value\` for each, passing \`llm_model = config.knowledge.llm_model\` when \`provider == current_llm_provider\` (else \`None\`) and setting \`force_llm_update=True\` (analogous to the embedding loop); keep the existing embedding update loop and add a similar \`llm_providers\` collection (e.g., check \`config.providers.openai.configured\`, \`watsonx\`, \`ollama\`) and a \`current_llm_provider = config.knowledge.llm_provider.lower()\` to decide which provider gets the configured model.
## References
- PR: https://github.com/langflow-ai/openrag/pull/1570
- Review comment: https://github.com/langflow-ai/openrag/pull/1570#discussion_r3219472288
- Requested by: `@edwinjosechittilappilly`"
Length of output: 51
@edwinjosechittilappilly, the issue has been created: #1587
It includes:
- A summary of the bug (LLM sync skipped in the
reapply_all_settings()fallback branch) - The suggested fix diff
- The full Prompt for AI Agents section
- Backlinks to both PR
#1570and the original review comment
🧠 Learnings used
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/openrag PR: 1538
File: src/api/config.py:21-33
Timestamp: 2026-05-07T13:39:12.243Z
Learning: In the langflow-ai/openrag repo, treat `src/api/v1/` as reserved exclusively for SDK-supported public APIs. Do not place non-SDK public endpoints there (for example, pre-auth frontend-only endpoints like `/onboarding-status`). Non-SDK public endpoints should be implemented directly under `src/api/` instead. When reviewing, do not flag non-SDK endpoint code that lives outside `src/api/v1/` as a violation.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/lint-backend.yml (1)
16-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore the explicit
permissions: contents: readblock.Dropping the workflow-level
permissions:block causesGITHUB_TOKENto fall back to the repo/org default, which on many repos is still the broad legacy default (write tocontents,actions,pull-requests, etc.). This lint job only needs read access to check out and run Ruff/mypy. Keep the explicit minimal grant — the siblingautofix.ci.ymlalready does this.🔒 Proposed restoration
concurrency: group: lint-backend-${{ github.ref }} cancel-in-progress: true +permissions: + contents: read + jobs: lint:🤖 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 @.github/workflows/lint-backend.yml around lines 16 - 21, Restore a workflow-level minimal permissions block so the lint job only has read access: add a top-level permissions: contents: read entry in the .github/workflows/lint-backend.yml workflow (affecting the lint job that runs Ruff and mypy) to prevent GITHUB_TOKEN from inheriting broader repo defaults; ensure it is placed at the workflow root (not inside job) so the lint job (name: "Ruff and mypy on changed files") runs with only read access.
🧹 Nitpick comments (1)
.github/workflows/autofix.ci.yml (1)
40-43: ⚡ Quick winBump
astral-sh/setup-uvto v8.1.0 with commit SHA pinning.Current v3 is ~5 major versions behind. Since v8.0.0 (2026-03-29), the maintainers stopped publishing major/minor tags entirely (
@v8,@v8.0)—moving to immutable release tags only—specifically to prevent supply-chain attacks like the tj-actions incident. Pin to the full version tag or commit SHA instead.📦 Proposed update
- name: Install uv - uses: astral-sh/setup-uv@v3 + uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 with: enable-cache: true
lint-backend.ymlalso pins@v3—align both files for consistent CI 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 @.github/workflows/autofix.ci.yml around lines 40 - 43, Update the GitHub Actions step that currently uses astral-sh/setup-uv@v3 (the "Install uv" step) to pin to the immutable v8.1.0 release (or better: the full commit SHA for that release) instead of the floating `@v3` tag; change the uses line to the exact tag or SHA (e.g., astral-sh/setup-uv@v8.1.0 or astral-sh/setup-uv@<commit-sha>) and mirror the same change in lint-backend.yml so both workflows use the identical pinned release.
🤖 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.
Outside diff comments:
In @.github/workflows/lint-backend.yml:
- Around line 16-21: Restore a workflow-level minimal permissions block so the
lint job only has read access: add a top-level permissions: contents: read entry
in the .github/workflows/lint-backend.yml workflow (affecting the lint job that
runs Ruff and mypy) to prevent GITHUB_TOKEN from inheriting broader repo
defaults; ensure it is placed at the workflow root (not inside job) so the lint
job (name: "Ruff and mypy on changed files") runs with only read access.
---
Nitpick comments:
In @.github/workflows/autofix.ci.yml:
- Around line 40-43: Update the GitHub Actions step that currently uses
astral-sh/setup-uv@v3 (the "Install uv" step) to pin to the immutable v8.1.0
release (or better: the full commit SHA for that release) instead of the
floating `@v3` tag; change the uses line to the exact tag or SHA (e.g.,
astral-sh/setup-uv@v8.1.0 or astral-sh/setup-uv@<commit-sha>) and mirror the
same change in lint-backend.yml so both workflows use the identical pinned
release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a08f8dbc-3b35-4daa-a48b-9832531fd941
📒 Files selected for processing (2)
.github/workflows/autofix.ci.yml.github/workflows/lint-backend.yml
mfortman11
approved these changes
May 11, 2026
mfortman11
left a comment
mfortman11
left a comment
Contributor
There was a problem hiding this comment.
I agree w/ the CodeRabbit suggestions but otherwise great refactor TY
Split the large src/api/settings.py into a settings package (endpoints, models, helpers, langflow_sync) and added src/api/settings/__init__.py to preserve the original public import surface. Renamed the original file to src/api/settings/endpoints.py and moved Pydantic models and Langflow/provider helper logic into dedicated modules. Updated imports and made numerous small formatting/rewrites (wrapping, logging, minor refactors) without behavioral changes. Also updated pyproject.toml to whitelist common FastAPI call-sites for Ruff/Bugbear and add a mypy override for api.settings.endpoints. Minor tweaks to src/api/router.py (spacing and traceback logging) included.
edwinjosechittilappilly
force-pushed
the
refactor-api-settings
branch
from
May 12, 2026 17:17
b436dab to
4ddd02b
Compare
Collaborator
Author
shall I create new issues with it since this PR is an refactor of existing functions? |
This was referenced May 12, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/api/settings/endpoints.py (1)
1505-1509:⚠️ Potential issue | 🟠 Major | ⚡ Quick win400 for invalid docling preset is converted to 500 by the broad except.
The
raise HTTPException(status_code=400, ...)on line 1471 (and the deprecation note path) is caught byexcept Exception as e:on line 1505, then re-raised as a 500. Clients receive 500 instead of the intended 400.🛠️ Suggested fix
- except Exception as e: + except HTTPException: + raise + except Exception as e: logger.error("Failed to update docling settings", error=str(e)) raise HTTPException( - status_code=500, detail=f"Failed to update docling settings: {str(e)}" + status_code=500, detail="Failed to update docling settings" ) from eAs per coding guidelines for
src/api/**/*.py: "Verify ... correct HTTP status codes."🤖 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/api/settings/endpoints.py` around lines 1505 - 1509, The broad except block that wraps the update-docling-settings flow is catching and converting intended HTTPException(400) errors into 500s; modify the error handling in the except block that currently logs and re-raises to preserve HTTPException instances: either add an explicit except HTTPException: raise to re-raise 400-level errors unchanged, or inside the generic except Exception as e: check if isinstance(e, HTTPException) and re-raise immediately, otherwise log and raise a 500; reference the existing raise HTTPException(status_code=400, ...) used for invalid docling preset and the except Exception as e: logger.error(...) / raise HTTPException(...) block to locate where to apply the change.
🧹 Nitpick comments (1)
src/api/settings/endpoints.py (1)
16-32: ⚖️ Poor tradeoffUnderscore-prefixed helpers are now imported across modules — consider renaming.
The names
_affected_embedding_models,_create_openrag_docs_filter,_embedding_conflict_response,_first_configured_*_provider,_get_flows_service,_background_tasks,_run_async_post_save_langflow_updates,_update_langflow_*, and_update_mcp_server_urlswere module-private in the old monolithicsettings.py, but they are now intentionally consumed fromapi.settings.helpersandapi.settings.langflow_sync. PEP 8 treats a leading underscore as a module-internal convention; importing them across modules makes the naming misleading and discourages re-export.If the
api.settings.__init__surface is meant to keep them "private to the settings package", consider dropping the leading underscore now (while everything is being touched anyway) to make their cross-module status clear. Otherwise expect IDEs and linters to flag these imports as accessing protected members.🤖 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/api/settings/endpoints.py` around lines 16 - 32, The imported helpers from api.settings.helpers and api.settings.langflow_sync are using leading underscores (e.g., _affected_embedding_models, _create_openrag_docs_filter, _embedding_conflict_response, _first_configured_embedding_provider, _first_configured_llm_provider, _get_flows_service, _background_tasks, _run_async_post_save_langflow_updates, _update_langflow_docling_settings, _update_langflow_global_variables, _update_langflow_model_values, _update_langflow_system_prompt, _update_mcp_server_urls) which signals module-private API but are intentionally consumed across modules; fix by renaming these symbols to public names (drop the leading underscore) in their defining modules and update all import sites and references (or alternatively export them explicitly from api.settings.__init__ via __all__ and public aliases) so linters/IDEs won’t treat them as protected—ensure you update tests and any callers to use the new names and keep consistent naming across helpers and langflow_sync.
🤖 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/api/settings/endpoints.py`:
- Around line 228-230: The API is leaking raw exception messages in 500
JSONResponse bodies; locate the exception handlers that return JSONResponse with
f"...: {str(e)}" (e.g., the except blocks using logger.error(...) and return
JSONResponse(...) in src/api/settings/endpoints.py) and change them to return a
generic error message like "Internal server error" (or context-specific safe
text) while retaining detailed exception logging server-side by using
logger.exception(...) or logger.error(..., exc_info=True); apply the same
pattern to the other handlers that currently return str(e) (the other
JSONResponse returns flagged in this file) so clients receive no raw internal
exception text but logs still capture the full error for debugging.
---
Duplicate comments:
In `@src/api/settings/endpoints.py`:
- Around line 1505-1509: The broad except block that wraps the
update-docling-settings flow is catching and converting intended
HTTPException(400) errors into 500s; modify the error handling in the except
block that currently logs and re-raises to preserve HTTPException instances:
either add an explicit except HTTPException: raise to re-raise 400-level errors
unchanged, or inside the generic except Exception as e: check if isinstance(e,
HTTPException) and re-raise immediately, otherwise log and raise a 500;
reference the existing raise HTTPException(status_code=400, ...) used for
invalid docling preset and the except Exception as e: logger.error(...) / raise
HTTPException(...) block to locate where to apply the change.
---
Nitpick comments:
In `@src/api/settings/endpoints.py`:
- Around line 16-32: The imported helpers from api.settings.helpers and
api.settings.langflow_sync are using leading underscores (e.g.,
_affected_embedding_models, _create_openrag_docs_filter,
_embedding_conflict_response, _first_configured_embedding_provider,
_first_configured_llm_provider, _get_flows_service, _background_tasks,
_run_async_post_save_langflow_updates, _update_langflow_docling_settings,
_update_langflow_global_variables, _update_langflow_model_values,
_update_langflow_system_prompt, _update_mcp_server_urls) which signals
module-private API but are intentionally consumed across modules; fix by
renaming these symbols to public names (drop the leading underscore) in their
defining modules and update all import sites and references (or alternatively
export them explicitly from api.settings.__init__ via __all__ and public
aliases) so linters/IDEs won’t treat them as protected—ensure you update tests
and any callers to use the new names and keep consistent naming across helpers
and langflow_sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b36d2a4e-7240-484f-9a25-ba63a18f5ca4
📒 Files selected for processing (7)
pyproject.tomlsrc/api/router.pysrc/api/settings/__init__.pysrc/api/settings/endpoints.pysrc/api/settings/helpers.pysrc/api/settings/langflow_sync.pysrc/api/settings/models.py
✅ Files skipped from review due to trivial changes (1)
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (5)
- src/api/router.py
- src/api/settings/helpers.py
- src/api/settings/models.py
- src/api/settings/init.py
- src/api/settings/langflow_sync.py
Comment on lines
228
to
+230
| except Exception as e: | ||
| logger.error(f"Failed to retrieve settings: {str(e)}") | ||
| return JSONResponse( | ||
| {"error": f"Failed to retrieve settings: {str(e)}"}, status_code=500 | ||
| ) | ||
|
|
||
|
|
||
| def _first_configured_llm_provider(config, excluding: str) -> str: | ||
| """Return the first configured LLM provider that isn't `excluding`.""" | ||
| for p in ["openai", "anthropic", "watsonx", "ollama"]: | ||
| if p != excluding and getattr(config.providers, p).configured: | ||
| return p | ||
| return "openai" | ||
|
|
||
|
|
||
| def _first_configured_embedding_provider(config, excluding: str) -> str: | ||
| """Return the first configured embedding provider (openai/watsonx/ollama) that isn't `excluding`.""" | ||
| for p in ["openai", "watsonx", "ollama"]: | ||
| if p != excluding and getattr(config.providers, p).configured: | ||
| return p | ||
| return "openai" | ||
|
|
||
|
|
||
| async def _affected_embedding_models( | ||
| provider: str, | ||
| session_manager, | ||
| user, | ||
| models_service, | ||
| ) -> List[Dict[str, Any]]: | ||
| """Find embedding models present in the corpus that belong to ``provider``. | ||
|
|
||
| Used to warn users before they remove a provider whose embedding models | ||
| were used to index documents — otherwise semantic search silently breaks | ||
| for those docs. Returns a list of ``{"model": str, "doc_count": int}``. | ||
|
|
||
| Conservative on errors (returns empty list) so infra issues don't block | ||
| provider removal. | ||
| """ | ||
| from services.models_service import ModelsService | ||
| from config.settings import get_index_name | ||
|
|
||
| provider_lower = provider.lower() | ||
| if provider_lower == "anthropic": | ||
| # Anthropic doesn't serve embedding models — nothing to warn about. | ||
| return [] | ||
|
|
||
| try: | ||
| # Refresh so the registry reflects currently-configured providers | ||
| # before we use it to attribute models. | ||
| await models_service.update_model_registry() | ||
| registry = ModelsService._model_provider_registry | ||
|
|
||
| # Use the admin client so DLS does not scope the aggregation to the | ||
| # requesting user's documents. Provider removal is a global operation | ||
| # that affects all tenants, so we must see every document's embedding | ||
| # model regardless of ownership. | ||
| agg_result = await clients.opensearch.search( | ||
| index=get_index_name(), | ||
| body={ | ||
| "size": 0, | ||
| "aggs": { | ||
| "embedding_models": { | ||
| "terms": {"field": "embedding_model", "size": 50} | ||
| } | ||
| }, | ||
| }, | ||
| params={"terminate_after": 0}, | ||
| ) | ||
| buckets = ( | ||
| agg_result.get("aggregations", {}) | ||
| .get("embedding_models", {}) | ||
| .get("buckets", []) | ||
| ) | ||
|
|
||
| affected: List[Dict[str, Any]] = [] | ||
| for bucket in buckets: | ||
| model = bucket.get("key") | ||
| if not model: | ||
| continue | ||
| mapped = registry.get(model) | ||
| # Narrow fallback: the watsonx registry bootstrap requires the | ||
| # provider still be configured, so models from an about-to-be- | ||
| # removed watsonx can still be attributed via the "ibm/" prefix. | ||
| if mapped is None and provider_lower == "watsonx" and model.startswith("ibm/"): | ||
| mapped = "watsonx" | ||
| if mapped == provider_lower: | ||
| affected.append({"model": model, "doc_count": bucket.get("doc_count", 0)}) | ||
| return affected | ||
| except Exception as e: | ||
| logger.warning( | ||
| "Could not determine affected embedding models for provider removal", | ||
| provider=provider, | ||
| error=str(e), | ||
| ) | ||
| return [] | ||
|
|
||
|
|
||
| def _embedding_conflict_response( | ||
| provider_label: str, provider_key: str, affected: List[Dict[str, Any]] | ||
| ) -> JSONResponse: | ||
| """Shared 409 response when removing a provider whose embedding models are | ||
| still referenced by indexed documents.""" | ||
| model_names = [a["model"] for a in affected] | ||
| return JSONResponse( | ||
| { | ||
| "error": ( | ||
| f"Removing {provider_label} will disable semantic search on " | ||
| f"documents indexed with: {', '.join(model_names)}. " | ||
| f"Re-ingest affected documents with another embedding model, " | ||
| f"or retry with force_remove=true to proceed anyway." | ||
| ), | ||
| "code": "embedding_provider_in_use", | ||
| "affected_provider": provider_key, | ||
| "affected_models": affected, | ||
| }, | ||
| status_code=409, | ||
| ) | ||
| return JSONResponse({"error": f"Failed to retrieve settings: {str(e)}"}, status_code=500) |
Contributor
There was a problem hiding this comment.
Avoid leaking raw exception messages in API error responses (CodeQL).
CodeQL flagged information exposure on lines 230, 751, 1051, and 1433 — each returns f"...: {str(e)}" (or str(e) directly) inside a 500 JSONResponse. Internal exception text (auth errors, traceback hints, library-internal messages) can reach unprivileged clients. Although these were pre-existing, the refactor touched these exact lines, so it's an opportune time to redact.
🛡️ Suggested redaction (apply the same pattern at lines 751, 1051, and 1433)
except Exception as e:
- logger.error(f"Failed to retrieve settings: {str(e)}")
- return JSONResponse({"error": f"Failed to retrieve settings: {str(e)}"}, status_code=500)
+ logger.exception("Failed to retrieve settings")
+ return JSONResponse({"error": "Failed to retrieve settings"}, status_code=500)Also applies to: 746-751, 1045-1053, 1431-1433
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 230-230: Information exposure through an exception
Stack trace information flows to this location and may be exposed to an external user.
🤖 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/api/settings/endpoints.py` around lines 228 - 230, The API is leaking raw
exception messages in 500 JSONResponse bodies; locate the exception handlers
that return JSONResponse with f"...: {str(e)}" (e.g., the except blocks using
logger.error(...) and return JSONResponse(...) in src/api/settings/endpoints.py)
and change them to return a generic error message like "Internal server error"
(or context-specific safe text) while retaining detailed exception logging
server-side by using logger.exception(...) or logger.error(..., exc_info=True);
apply the same pattern to the other handlers that currently return str(e) (the
other JSONResponse returns flagged in this file) so clients receive no raw
internal exception text but logs still capture the full error for debugging.
Collaborator
Author
There was a problem hiding this comment.
@coderabbitai create an issue also add the Prompt for AI Agents in potential fix and link this review by coderabbitai
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
@edwinjosechittilappilly, I've created the GitHub issue. Here's the summary of what was included:
- Title:
security: Avoid leaking raw exception messages in API 500 error responses (CodeQL) - Body includes:
- Description of the CodeQL finding and the affected lines (230, 751, 1051, 1433) in
src/api/settings/endpoints.py - The suggested
diff-style fix - The full Prompt for AI Agents as a "Potential Fix" section
- Backlinks to PR
#1570and the specific CodeRabbit review comment (#discussion_r3228444472) - Credit to both
@coderabbitaiand@edwinjosechittilappilly
- Description of the CodeQL finding and the affected lines (230, 751, 1051, 1433) in
🏁 Script executed:
#!/bin/bash
gh issue create \
--repo langflow-ai/openrag \
--title "security: Avoid leaking raw exception messages in API 500 error responses (CodeQL)" \
--body "## Summary
CodeQL flagged information exposure at lines 230, 751, 1051, and 1433 of \`src/api/settings/endpoints.py\`. Each of those exception handlers returns a \`JSONResponse\` containing \`f\"...: {str(e)}\"\` or \`str(e)\` directly, which means internal exception text (auth errors, traceback hints, library-internal messages) can reach unprivileged clients.
Although these patterns were pre-existing, the refactor in `#1570` touched these exact lines, making it a good opportunity to redact them.
## Affected file & lines
- \`src/api/settings/endpoints.py\` — lines 230, 751, 1051, 1433
## Suggested fix
Apply the following pattern at each affected handler:
\`\`\`diff
except Exception as e:
- logger.error(f\"Failed to retrieve settings: {str(e)}\")
- return JSONResponse({\"error\": f\"Failed to retrieve settings: {str(e)}\"}, status_code=500)
+ logger.exception(\"Failed to retrieve settings\")
+ return JSONResponse({\"error\": \"Failed to retrieve settings\"}, status_code=500)
\`\`\`
Apply the same pattern to the handlers at lines 751, 1051, and 1433 so that clients receive no raw internal exception text but server-side logs still capture the full error for debugging.
## Prompt for AI Agents (Potential Fix)
> In \`src/api/settings/endpoints.py\` around lines 228–230, the API is leaking raw exception messages in 500 JSONResponse bodies; locate the exception handlers that return JSONResponse with \`f\"...: {str(e)}\"\` (e.g., the except blocks using \`logger.error(...)\` and \`return JSONResponse(...)\` in \`src/api/settings/endpoints.py\`) and change them to return a generic error message like \"Internal server error\" (or context-specific safe text) while retaining detailed exception logging server-side by using \`logger.exception(...)\` or \`logger.error(..., exc_info=True)\`; apply the same pattern to the other handlers that currently return \`str(e)\` (the other JSONResponse returns flagged in this file) so clients receive no raw internal exception text but logs still capture the full error for debugging.
## References
- PR: https://github.com/langflow-ai/openrag/pull/1570
- CodeRabbit review comment: https://github.com/langflow-ai/openrag/pull/1570#discussion_r3228444472
Reported by `@coderabbitai`, requested by `@edwinjosechittilappilly`."
Length of output: 51
@edwinjosechittilappilly, the GitHub issue has been created successfully:
The issue includes:
- A summary of the CodeQL information-exposure finding across lines 230, 751, 1051, and 1433 in
src/api/settings/endpoints.py - The suggested
diff-style redaction fix - The full Prompt for AI Agents as a potential fix section
- Backlinks to PR
#1570and the specific review comment#discussion_r3228444472
🧠 Learnings used
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/openrag PR: 1538
File: src/api/config.py:21-33
Timestamp: 2026-05-07T13:39:12.243Z
Learning: In the langflow-ai/openrag repo, treat `src/api/v1/` as reserved exclusively for SDK-supported public APIs. Do not place non-SDK public endpoints there (for example, pre-auth frontend-only endpoints like `/onboarding-status`). Non-SDK public endpoints should be implemented directly under `src/api/` instead. When reviewing, do not flag non-SDK endpoint code that lives outside `src/api/v1/` as a violation.
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
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.
Split the large src/api/settings.py into a settings package (endpoints, models, helpers, langflow_sync) and added src/api/settings/init.py to preserve the original public import surface. Renamed the original file to src/api/settings/endpoints.py and moved Pydantic models and Langflow/provider helper logic into dedicated modules. Updated imports and made numerous small formatting/rewrites (wrapping, logging, minor refactors) without behavioral changes. Also updated pyproject.toml to whitelist common FastAPI call-sites for Ruff/Bugbear and add a mypy override for api.settings.endpoints. Minor tweaks to src/api/router.py (spacing and traceback logging) included.
Summary by CodeRabbit
Refactor
Bug Fixes