Added ollama component#3664
Open
onesketchyguy wants to merge 4 commits into
Open
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Note
|
| Layer / File(s) | Summary |
|---|---|
Configuration and Ollama service src/assets/env.js, src/environments/*, src/app/settings/settings.service.ts, src/app/shared/services/ollama.service.ts |
Adds Ollama configuration fallbacks, localStorage persistence, model discovery, generation, single-SELECT validation, and connection checks. |
External Services views and configuration src/app/system/external-services/ollama/*, src/app/system/external-services/external-services.component.*, src/app/system/system-routing.module.ts |
Adds Ollama overview and edit screens, model loading, connection testing, settings submission, navigation, and routes. |
Create Report AI SQL flow src/app/system/manage-reports/create-report/*, src/assets/translations/en-US.json |
Adds an Ollama-gated prompt panel that generates, previews, and inserts SQL into the report form. |
Bracket placement formatting src/app/analytics/..., src/app/clients/..., src/app/configuration-wizard/..., src/app/core/error-handler/README.md, src/app/organization/..., src/app/products/..., src/app/system/manage-data-tables/..., src/app/system/manage-jobs/... |
Reformats closing brackets and tuple annotations without changing logic or behavior. |
Estimated code review effort: 4 (Complex) | ~60 minutes
Sequence Diagram(s)
sequenceDiagram
participant User
participant CreateReportComponent
participant OllamaService
participant OllamaServer
User->>CreateReportComponent: enter prompt and generate SQL
CreateReportComponent->>OllamaService: generateSqlQuery(prompt)
OllamaService->>OllamaServer: POST /api/generate
OllamaServer-->>OllamaService: response text
OllamaService-->>CreateReportComponent: validated SQL
User->>CreateReportComponent: use generated SQL
CreateReportComponent->>CreateReportComponent: update reportSql control
sequenceDiagram
participant User
participant EditOllamaComponent
participant OllamaService
participant SettingsService
User->>EditOllamaComponent: enter URL and test connection
EditOllamaComponent->>OllamaService: checkConnectionAt(url)
OllamaService-->>EditOllamaComponent: connection status
EditOllamaComponent->>OllamaService: listModelsFromUrl(url)
OllamaService-->>EditOllamaComponent: model names
User->>EditOllamaComponent: submit settings
EditOllamaComponent->>SettingsService: persist settings
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title matches the PR’s Ollama integration work, though it is narrower than the full external-service/report-generation change. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
src/app/system/manage-reports/create-report/create-report.component.scss (1)
20-25: ⚡ Quick winAlign spacing/colors with shared SCSS tokens and 8px grid.
This block introduces explicit pixel/radius/color values (
4px, hex colors, custom rgba) and off-grid spacing (0.75rem) instead of shared theme variables/tokens. Please switch these to existing variables fromsrc/main.scss/src/theme/mifosx-theme.scssand 8px-based spacing values.As per coding guidelines, “Stick to the 8px grid system for visual design and spacing” and “Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values.”Also applies to: 31-36, 44-61
🤖 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/app/system/manage-reports/create-report/create-report.component.scss` around lines 20 - 25, Replace the hardcoded spacing, border-radius, and color values in the create-report.component.scss with shared SCSS variables from src/main.scss and src/theme/mifosx-theme.scss. Specifically, convert margin-top from 0.75rem to an 8px-grid-aligned value, replace the border-radius 4px with an appropriate theme variable, and swap the hardcoded rgba color values (rgb(0 0 0 / 12%) and rgb(0 0 0 / 2%)) with existing theme color tokens. Apply the same variable-based approach to the additional blocks at lines 31-36 and 44-61.Source: Coding guidelines
src/app/system/external-services/external-services.component.html (1)
98-102: ⚡ Quick winUse a button for the arrow toggle control.
At Line 98 and Line 101, the clickable
<fa-icon>is mouse-centric. Replacing it with a focusable button keeps toggle behavior keyboard-accessible.Suggested change
- `@if` (!arrowBooleans[4]) { - <fa-icon (click)="arrowBooleansToggle(4); $event.stopPropagation()" icon="arrow-down" size="md"></fa-icon> - } - `@if` (arrowBooleans[4]) { - <fa-icon (click)="arrowBooleansToggle(4); $event.stopPropagation()" icon="arrow-up" size="md"></fa-icon> - } + <button + mat-icon-button + type="button" + (click)="arrowBooleansToggle(4); $event.stopPropagation()" + [attr.aria-label]="arrowBooleans[4] ? ('labels.buttons.Collapse' | translate) : ('labels.buttons.Expand' | translate)"> + <fa-icon [icon]="arrowBooleans[4] ? 'arrow-up' : 'arrow-down'" size="md"></fa-icon> + </button>🤖 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/app/system/external-services/external-services.component.html` around lines 98 - 102, Replace the two clickable fa-icon elements (one with icon="arrow-down" at line 98 and one with icon="arrow-up" at line 101) with proper button elements to ensure keyboard accessibility. Wrap each fa-icon in a button element, move the (click)="arrowBooleansToggle(4); $event.stopPropagation()" handler from the fa-icon to the button, and keep the fa-icon nested inside the button with its icon and size attributes. This ensures the toggle control is accessible via both mouse and keyboard.
🤖 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/app/shared/services/ollama.service.ts`:
- Around line 85-109: The generateSqlQuery method currently strips markdown from
the generated SQL but does not validate that it contains only a single SELECT
statement. After calling stripMarkdownFences(sql) in the pipe chain, add
validation logic to ensure the stripped SQL contains only a single SELECT
statement with no additional commands or arbitrary content. Implement this
validation before the observable returns the SQL to the caller to prevent
invalid SQL from being used in report creation.
- Around line 124-127: The checkConnectionAt method has a logic error where the
condition models.length >= 0 is always true for any array, including empty ones
returned on connection failures. Fix this by changing the comparison operator
from >= to > in the map operator within the pipe, so that the method only
returns true when models.length > 0, indicating a successful connection with
actual models retrieved from the URL.
In
`@src/app/system/external-services/ollama/edit-ollama/edit-ollama.component.html`:
- Line 20: The placeholder attributes on the input elements in
edit-ollama.component.html contain hardcoded strings that will not be
translated. Replace the hardcoded placeholder text on line 20 (the URL
placeholder "http://192.168.1.100:11434" in the formControlName="url" input) and
line 38 (the model placeholder) with i18n variables using the appropriate i18n
pipe or attribute syntax to ensure these user-facing strings are properly
localized according to the coding guidelines for src/app/**/*.html files.
In
`@src/app/system/external-services/ollama/edit-ollama/edit-ollama.component.ts`:
- Around line 93-105: The testConnection method in the edit-ollama component is
calling checkConnectionAt(url) but the service implementation returns true
whenever models.length >= 0, which is always true for arrays and prevents proper
failure detection. Update the checkConnectionAt method in the ollama.service.ts
file to properly return false when the HTTP connection to the URL actually fails
or returns an error, rather than only checking if the models array has a valid
length. This ensures the failure branch in the subscribe callback (showing the
"Ollama connection failed" alert) can actually execute for unreachable URLs.
- Line 10: Replace the untyped reactive forms imports (UntypedFormBuilder and
UntypedFormGroup) with their typed equivalents (FormBuilder and FormGroup).
Define a form type interface or use an inline type annotation for FormGroup that
specifies the form structure with FormControl properties for enabled (boolean),
url (string), and model (string). Update all references to UntypedFormBuilder
and UntypedFormGroup throughout the component (including lines 36 and 45) to use
the typed versions with the form type applied.
In `@src/app/system/external-services/ollama/ollama.component.scss`:
- Around line 12-25: The styles in the hint container and hint-icon class
sections use hardcoded literal values (0.75rem, 4px, rgb/hex colors, 1.1rem)
instead of shared design tokens. Replace these ad-hoc values with SCSS variables
from src/main.scss and src/theme/mifosx-theme.scss: the padding values (0.75rem,
1rem) and gap (0.5rem) should use spacing tokens aligned to the 8px grid, the
border-radius (4px) should use a shared radius token, the background color
(rgb(63 81 181 / 8%)), text color (rgb(0 0 0 / 70%)), and icon color (`#3f51b5`)
should use shared color tokens from the theme, and the font-size values (0.9rem,
1.1rem) should reference shared typography tokens to maintain consistency across
the project.
In `@src/app/system/external-services/ollama/ollama.component.ts`:
- Around line 69-72: The rows array in the component initializes the enabled
property with hardcoded string literals 'true' and 'false', which will not be
localized for non-English UIs. Replace these hardcoded strings with i18n
variables or localized strings through your project's internationalization
service. Specifically, update the value assignment for the 'enabled' row entry
to use the appropriate i18n reference instead of the ternary operator returning
'true' or 'false' literals.
In `@src/app/system/manage-reports/create-report/create-report.component.scss`:
- Line 57: The CSS property word-break: break-word is deprecated and does not
properly handle long unbreakable strings according to modern CSS standards.
Replace the word-break: break-word property in the create-report.component.scss
file with overflow-wrap: anywhere; to achieve the same visual result while using
the current CSS standard approach for wrapping long text.
---
Nitpick comments:
In `@src/app/system/external-services/external-services.component.html`:
- Around line 98-102: Replace the two clickable fa-icon elements (one with
icon="arrow-down" at line 98 and one with icon="arrow-up" at line 101) with
proper button elements to ensure keyboard accessibility. Wrap each fa-icon in a
button element, move the (click)="arrowBooleansToggle(4);
$event.stopPropagation()" handler from the fa-icon to the button, and keep the
fa-icon nested inside the button with its icon and size attributes. This ensures
the toggle control is accessible via both mouse and keyboard.
In `@src/app/system/manage-reports/create-report/create-report.component.scss`:
- Around line 20-25: Replace the hardcoded spacing, border-radius, and color
values in the create-report.component.scss with shared SCSS variables from
src/main.scss and src/theme/mifosx-theme.scss. Specifically, convert margin-top
from 0.75rem to an 8px-grid-aligned value, replace the border-radius 4px with an
appropriate theme variable, and swap the hardcoded rgba color values (rgb(0 0 0
/ 12%) and rgb(0 0 0 / 2%)) with existing theme color tokens. Apply the same
variable-based approach to the additional blocks at lines 31-36 and 44-61.
🪄 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: b07e8299-2df9-499f-8524-9fb98a21fcc6
📒 Files selected for processing (31)
src/app/analytics/services/analytics-data-source.service.tssrc/app/clients/client-stepper/client-family-members-step/client-family-member-dialog/client-family-member-dialog.component.tssrc/app/clients/clients-view/client-actions/client-action-notifier.service.spec.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.spec.tssrc/app/clients/clients-view/family-members-tab/add-family-member/add-family-member.component.tssrc/app/clients/clients-view/family-members-tab/edit-family-member/edit-family-member.component.tssrc/app/clients/create-client/create-client.component.spec.tssrc/app/configuration-wizard/popover/popover.service.tssrc/app/core/error-handler/README.mdsrc/app/organization/payment-types/edit-payment-type/edit-payment-type.component.tssrc/app/products/floating-rates/floating-rate-period-dialog/floating-rate-period-dialog.component.tssrc/app/settings/settings.service.tssrc/app/shared/services/ollama.service.tssrc/app/system/external-services/external-services.component.htmlsrc/app/system/external-services/external-services.component.tssrc/app/system/external-services/ollama/edit-ollama/edit-ollama.component.htmlsrc/app/system/external-services/ollama/edit-ollama/edit-ollama.component.scsssrc/app/system/external-services/ollama/edit-ollama/edit-ollama.component.tssrc/app/system/external-services/ollama/ollama.component.htmlsrc/app/system/external-services/ollama/ollama.component.scsssrc/app/system/external-services/ollama/ollama.component.tssrc/app/system/manage-data-tables/column-dialog/column-dialog.component.tssrc/app/system/manage-jobs/workflow-jobs/workflow-diagram/workflow-diagram.component.tssrc/app/system/manage-reports/create-report/create-report.component.htmlsrc/app/system/manage-reports/create-report/create-report.component.scsssrc/app/system/manage-reports/create-report/create-report.component.tssrc/app/system/system-routing.module.tssrc/assets/env.jssrc/assets/translations/en-US.jsonsrc/environments/environment.prod.tssrc/environments/environment.ts
Comment on lines
+12
to
+25
| gap: 0.5rem; | ||
| padding: 0.75rem 1rem; | ||
| background: rgb(63 81 181 / 8%); | ||
| border-radius: 4px; | ||
| margin: 0; | ||
| font-size: 0.9rem; | ||
| color: rgb(0 0 0 / 70%); | ||
| } | ||
|
|
||
| .hint-icon { | ||
| font-size: 1.1rem; | ||
| width: 1.1rem; | ||
| height: 1.1rem; | ||
| color: #3f51b5; |
There was a problem hiding this comment.
Align hint styling with shared spacing/color tokens.
Lines 13-16 and 22-25 use ad-hoc spacing/radius/color literals (0.75rem, 4px, hex/rgb values), which drifts from the project’s shared design-token contract.
As per coding guidelines, src/**/*.{scss,html} must follow the 8px grid and leverage SCSS variables from src/main.scss and src/theme/mifosx-theme.scss.
🤖 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/app/system/external-services/ollama/ollama.component.scss` around lines
12 - 25, The styles in the hint container and hint-icon class sections use
hardcoded literal values (0.75rem, 4px, rgb/hex colors, 1.1rem) instead of
shared design tokens. Replace these ad-hoc values with SCSS variables from
src/main.scss and src/theme/mifosx-theme.scss: the padding values (0.75rem,
1rem) and gap (0.5rem) should use spacing tokens aligned to the 8px grid, the
border-radius (4px) should use a shared radius token, the background color
(rgb(63 81 181 / 8%)), text color (rgb(0 0 0 / 70%)), and icon color (`#3f51b5`)
should use shared color tokens from the theme, and the font-size values (0.9rem,
1.1rem) should reference shared typography tokens to maintain consistency across
the project.
Source: Coding guidelines
IOhacker
requested changes
Jun 19, 2026
IOhacker
left a comment
IOhacker
left a comment
Contributor
There was a problem hiding this comment.
Hello, there is an ongoing effort for implementing a Mifos copilot features. @shubhamkumar9199 could you please review
IOhacker
requested changes
Jun 21, 2026
| * real table and column names. | ||
| */ | ||
| generateSqlQuery(description: string): Observable<string> { | ||
| const system = `You are a Mifos Fineract MySQL report query generator. |
Contributor
There was a problem hiding this comment.
Mifos can use MySQL, MariaDB or Postgresql. I suggest to use ANSI SQL
| @@ -0,0 +1,65 @@ | |||
| <!-- | |||
| Copyright since 2025 Mifos Initiative | |||
Contributor
There was a problem hiding this comment.
Could you please add a Video of the feature implemented?
| @@ -88,6 +88,24 @@ <h4 matLine [routerLink]="['notification']"> | |||
| </p> | |||
| } | |||
| </mat-list-item> | |||
Contributor
There was a problem hiding this comment.
Ollama exposes an OpenAPI compatible API, so then it should not be fixed to Ollama. Because it can implement other SaaS models providers
|
👋 Hi @onesketchyguy — thank you for your pull request. This PR is currently blocked because we do not have a Contributor License Agreement (CLA) on file for your GitHub account. To get unblocked:
|
mifos-cla-check
Bot
added
the
cla-required
|
👋 Hi @onesketchyguy — thank you for your pull request. This PR is currently blocked because we do not have a Contributor License Agreement (CLA) on file for your GitHub account. To get unblocked:
|
Author
|
I have not created a video yet but I have addressed the complaints above. |
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/assets/translations/en-US.json`:
- Line 757: Remove the duplicate translation entries identified throughout
en-US.json, including undo_account_transfer and all additionally reported
locations, while retaining one canonical entry per key within each containing
object and preserving its intended translation value.
🪄 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: c58021e9-7edb-45ea-b3f9-f00efaf90224
📒 Files selected for processing (5)
src/app/settings/settings.service.tssrc/assets/env.jssrc/assets/translations/en-US.jsonsrc/environments/environment.prod.tssrc/environments/environment.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/assets/env.js
- src/environments/environment.ts
- src/environments/environment.prod.ts
- src/app/settings/settings.service.ts
| "Transaction Reverted": "Transaction Reverted", | ||
| "Transfer Client": "Transfer Client", | ||
| "Transfer Clients": "Transfer Clients", | ||
| "Unassign Staff": "Unassign Staff", | ||
| "Undo": "Undo", | ||
| "Undo Rejection": "Undo Rejection", | ||
| "Undo Transfer": "Undo Transfer", | ||
| "undo_account_transfer": "Undo Account Transfer", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicate translation keys before merging.
Biome reports duplicate object keys throughout this file. These entries can shadow earlier translations and currently produce lint errors, so the branch may fail validation. Keep one entry per key in each containing object.
Also applies to: 1634-1634, 1653-1653, 1776-1776, 1779-1779, 1802-1802, 1969-1969, 2084-2084, 2114-2114, 2123-2123, 2197-2197, 2200-2200, 2257-2257, 2387-2387, 2390-2390, 2486-2486, 2532-2532, 2553-2553, 2630-2630, 2775-2775, 2904-2906, 3026-3026, 3050-3050, 3139-3139, 3466-3466, 3534-3534, 3672-3673, 3699-3699, 3756-3756, 3760-3760, 3778-3778, 3803-3804, 3809-3809, 3815-3815, 3824-3825, 3832-3832, 3974-3974, 4008-4008, 4043-4043, 4053-4053, 4056-4056, 4062-4063, 4075-4076, 4117-4117, 4152-4152, 4160-4160, 4215-4215, 4220-4220, 4251-4251, 4261-4261, 4263-4263, 4331-4332
🧰 Tools
🪛 Biome (2.5.3)
[error] 757-757: The key undo_account_transfer was already declared.
(lint/suspicious/noDuplicateObjectKeys)
🤖 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/assets/translations/en-US.json` at line 757, Remove the duplicate
translation entries identified throughout en-US.json, including
undo_account_transfer and all additionally reported locations, while retaining
one canonical entry per key within each containing object and preserving its
intended translation value.
Source: Linters/SAST tools
|
👋 Hi @onesketchyguy — thank you for your pull request. This PR is currently blocked because we do not have a Contributor License Agreement (CLA) on file for your GitHub account. To get unblocked:
|
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Added ollama to the external services, allowing admins to use it to generate reports.
Summary by CodeRabbit
Release Notes
New Features
Style