Added ollama component#3664
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Environment config and SettingsService persistence src/assets/env.js, src/environments/environment.ts, src/environments/environment.prod.ts, src/app/settings/settings.service.ts |
Defines ollamaEnabled, ollamaUrl, ollamaModel in environment files with window.env defaults. Adds setters and getters in SettingsService persisting values under mifosXOllama* localStorage keys with environment fallbacks. |
OllamaService HTTP layer src/app/shared/services/ollama.service.ts |
New OllamaService uses HttpBackend to bypass auth interceptors. Implements listModels, listModelsFromUrl, generate, generateSqlQuery (with embedded Fineract schema system prompt and Markdown fence stripping), checkConnection, and checkConnectionAt. |
OllamaComponent — settings read-only view src/app/system/external-services/ollama/ollama.component.ts, ...ollama.component.html, ...ollama.component.scss |
Standalone component renders a sortable Material table of enabled/url/model rows from SettingsService, with a usage hint panel when enabled. |
EditOllamaComponent — settings edit form src/app/system/external-services/ollama/edit-ollama/edit-ollama.component.ts, ...html, ...scss |
Reactive form with enabled toggle, URL input (required when enabled), conditional model dropdown or free-text. Implements syncUrlValidators, loadModels, testConnection (alerts via AlertService), and submit (persists via SettingsService). |
Routing and External Services navigation src/app/system/system-routing.module.ts, src/app/system/external-services/external-services.component.ts, ...external-services.component.html |
Adds ollama and ollama/edit child routes under external-services. Extends arrowBooleans to 5 entries and adds a new nav list item for the Ollama route. |
AI SQL generation panel in Create Report src/app/system/manage-reports/create-report/create-report.component.ts, ...html, ...scss |
Adds ollamaEnabled getter, aiPrompt form control, generateAiSql() calling OllamaService.generateSqlQuery, and useGeneratedSql() writing output to reportSql. Template adds a collapsible panel with textarea, spinner, SQL preview, and error display. |
Code Formatting Cleanup
| Layer / File(s) | Summary |
|---|---|
Bracket placement formatting across unrelated files src/app/analytics/services/analytics-data-source.service.ts, src/app/clients/..., src/app/configuration-wizard/popover/popover.service.ts, src/app/core/error-handler/README.md, src/app/organization/payment-types/edit-payment-type/edit-payment-type.component.ts, src/app/products/floating-rates/floating-rate-period-dialog/floating-rate-period-dialog.component.ts, src/app/system/manage-data-tables/column-dialog/column-dialog.component.ts, src/app/system/manage-jobs/workflow-jobs/workflow-diagram/workflow-diagram.component.ts |
Closing brackets for array literals and tuple type annotations moved inline. No logic or behavior changed in any file. |
Sequence Diagram(s)
sequenceDiagram
rect rgba(173, 216, 230, 0.5)
note over User,OllamaServer: AI SQL Generation (Create Report)
end
participant User
participant CreateReportComponent
participant OllamaService
participant OllamaServer
User->>CreateReportComponent: enter natural-language prompt
User->>CreateReportComponent: click Generate SQL (or Ctrl+Enter)
CreateReportComponent->>CreateReportComponent: set aiLoading=true, reset aiSql/aiError
CreateReportComponent->>OllamaService: generateSqlQuery(prompt)
OllamaService->>OllamaService: build Fineract schema system prompt
OllamaService->>OllamaServer: POST /api/generate
OllamaServer-->>OllamaService: raw text response
OllamaService->>OllamaService: stripMarkdownFences()
OllamaService-->>CreateReportComponent: SQL string
CreateReportComponent->>CreateReportComponent: set aiSql, aiLoading=false, markForCheck()
User->>CreateReportComponent: click Use this SQL
CreateReportComponent->>CreateReportComponent: write aiSql → reportSql control, aiExpanded=false
sequenceDiagram
rect rgba(144, 238, 144, 0.5)
note over User,SettingsService: Ollama Settings Configuration
end
participant User
participant EditOllamaComponent
participant OllamaService
participant AlertService
participant SettingsService
User->>EditOllamaComponent: toggle enabled, enter URL
EditOllamaComponent->>EditOllamaComponent: syncUrlValidators(enabled)
User->>EditOllamaComponent: click Test Connection
EditOllamaComponent->>OllamaService: checkConnectionAt(url)
OllamaService-->>EditOllamaComponent: boolean
EditOllamaComponent->>AlertService: show success/failure alert
EditOllamaComponent->>OllamaService: listModelsFromUrl(url)
OllamaService-->>EditOllamaComponent: string[]
User->>EditOllamaComponent: select model, click Submit
EditOllamaComponent->>SettingsService: setOllamaEnabled / setOllamaUrl / setOllamaModel
EditOllamaComponent->>EditOllamaComponent: navigate back to /ollama
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
- adamsaghy
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | The title 'Added ollama component' is partially related to the changeset but misleading, as the PR adds comprehensive Ollama AI integration (services, settings, routing, environment config, multiple components) not just a single component. | Revise the title to reflect the full scope, such as 'Add Ollama AI Report Assistant integration' or 'Integrate Ollama service for AI-powered report generation'. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| 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. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ 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 and usage tips.
Warning
![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
| generateSqlQuery(description: string): Observable<string> { | ||
| const system = `You are a Mifos Fineract MySQL report query generator. | ||
| Generate a single MySQL SELECT query based on the user's description. | ||
| Use only these tables and columns: | ||
|
|
||
| m_client: id, account_no, firstname, lastname, status_enum (100=pending 300=active 600=closed), office_id, staff_id, activation_date, birthdate, mobile_no, email_address | ||
| m_loan: id, account_no, client_id, group_id, product_id, loan_officer_id, loan_status_id (100=pending 200=approved 300=active 400=withdrawn 500=rejected 600=closed 700=overpaid), principal_amount, approved_principal, outstanding_derived, disbursedon_date, expected_maturedon_date, closedon_date, currency_code, office_id | ||
| m_loan_repayment_schedule: id, loan_id, duedate, installment, principal_amount, interest_amount, fee_charges_amount, penalty_charges_amount, principal_completed_derived, interest_completed_derived, completed_derived | ||
| m_loan_transaction: id, loan_id, transaction_type_enum (1=disbursement 2=repayment 5=waive_interest 9=written_off), transaction_date, amount, is_reversed | ||
| m_savings_account: id, account_no, client_id, group_id, product_id, status_enum (100=submitted 200=approved 300=active 600=closed), account_balance_derived, currency_code, office_id | ||
| m_savings_account_transaction: id, savings_account_id, transaction_type_enum (1=deposit 2=withdrawal 4=interest_posting), transaction_date, amount, running_balance_derived, is_reversed | ||
| m_office: id, name, hierarchy, parent_id, opening_date | ||
| m_staff: id, firstname, lastname, display_name, is_loan_officer, is_active, office_id | ||
| m_group: id, account_no, display_name, office_id, staff_id, status_enum, level_id, activation_date | ||
| m_product_loan: id, name, currency_code, min_principal_amount, max_principal_amount | ||
| m_savings_product: id, name, currency_code | ||
| m_currency: code, name, display_symbol, decimal_places | ||
|
|
||
| Rules: | ||
| - Return ONLY the raw SQL SELECT statement. No markdown, no backticks, no explanation. | ||
| - Use meaningful column aliases (e.g. c.firstname AS first_name). | ||
| - Parameterise date ranges with ${'{'}dateFormat${'}'} and ${'{'}startDate${'}'} / ${'{'}endDate${'}'} placeholders if the user mentions a date range.`; | ||
|
|
||
| return this.generate(description, system).pipe(map((sql) => this.stripMarkdownFences(sql))); | ||
| } |
There was a problem hiding this comment.
Validate generated SQL before returning it to the form.
The current flow strips markdown but still accepts arbitrary model output. Add a guard to enforce a single SELECT statement before exposing SQL to report creation.
Suggested fix
generateSqlQuery(description: string): Observable<string> {
@@
- return this.generate(description, system).pipe(map((sql) => this.stripMarkdownFences(sql)));
+ return this.generate(description, system).pipe(
+ map((sql) => this.validateSelectSql(this.stripMarkdownFences(sql)))
+ );
}
@@
private stripMarkdownFences(text: string): string {
@@
}
+
+ private validateSelectSql(sql: string): string {
+ const normalized = sql.trim().replace(/;+\s*$/, '');
+ const isSingleSelect = /^select\b/i.test(normalized) && !/;\s*\S/.test(normalized);
+ return isSingleSelect ? normalized : '';
+ }Also applies to: 111-116
🤖 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/shared/services/ollama.service.ts` around lines 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.
| checkConnectionAt(url: string): Observable<boolean> { | ||
| if (!url) return of(false); | ||
| return this.listModelsFromUrl(url).pipe(map((models) => models.length >= 0)); | ||
| } |
There was a problem hiding this comment.
checkConnectionAt currently returns true for any non-empty URL.
models.length >= 0 is always true, so failures converted to [] in listModelsFromUrl() still report a successful connection.
Suggested fix
checkConnectionAt(url: string): Observable<boolean> {
if (!url) return of(false);
- return this.listModelsFromUrl(url).pipe(map((models) => models.length >= 0));
+ return this.externalHttp.get<OllamaTagsResponse>(`${url}/api/tags`).pipe(
+ map(() => true),
+ catchError(() => of(false))
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| checkConnectionAt(url: string): Observable<boolean> { | |
| if (!url) return of(false); | |
| return this.listModelsFromUrl(url).pipe(map((models) => models.length >= 0)); | |
| } | |
| checkConnectionAt(url: string): Observable<boolean> { | |
| if (!url) return of(false); | |
| return this.externalHttp.get<OllamaTagsResponse>(`${url}/api/tags`).pipe( | |
| map(() => true), | |
| catchError(() => of(false)) | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/shared/services/ollama.service.ts` around lines 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.
|
|
||
| <mat-form-field> | ||
| <mat-label>{{ 'labels.inputs.Ollama URL' | translate }}</mat-label> | ||
| <input matInput formControlName="url" placeholder="http://192.168.1.100:11434" /> |
There was a problem hiding this comment.
Localize the URL/model placeholders.
Line 20 and Line 38 use hardcoded placeholder text, so these strings won’t be translated.
As per coding guidelines, src/app/**/*.{html,ts} should use i18n variables for all user-facing strings instead of hardcoded text.
Also applies to: 38-38
🤖 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/edit-ollama/edit-ollama.component.html`
at 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.
Source: Coding guidelines
| */ | ||
|
|
||
| import { ChangeDetectionStrategy, ChangeDetectorRef, Component, OnInit, inject } from '@angular/core'; | ||
| import { UntypedFormBuilder, UntypedFormGroup, Validators } from '@angular/forms'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify strict-typing intent and current untyped-form usage in this feature area.
fd -i 'tsconfig*.json' .
rg -n '"strict"|"strictTemplates"|"strictNullChecks"|"strictPropertyInitialization"' tsconfig*.json
rg -n 'UntypedFormBuilder|UntypedFormGroup|UntypedFormControl' src/app/system/external-services/ollamaRepository: openMF/web-app
Length of output: 528
🏁 Script executed:
#!/bin/bash
# Check main tsconfig.json strict settings
head -30 tsconfig.json
# Check Angular version
rg -A 2 '"`@angular/core`"' package.json | head -5
# Read lines 45-56 to see full scope
sed -n '40,60p' src/app/system/external-services/ollama/edit-ollama/edit-ollama.component.tsRepository: openMF/web-app
Length of output: 1473
🏁 Script executed:
#!/bin/bash
# Check if typed forms are used elsewhere in the codebase
rg -n 'FormGroup<|FormBuilder.*group<|FormControl<' src/app/system/external-services --count=20
# Check the complete component structure
sed -n '1,80p' src/app/system/external-services/ollama/edit-ollama/edit-ollama.component.ts
# Check imports and interfaces
sed -n '1,40p' src/app/system/external-services/ollama/edit-ollama/edit-ollama.component.tsRepository: openMF/web-app
Length of output: 4704
🏁 Script executed:
#!/bin/bash
# Check for typed forms usage in the codebase to understand the pattern
rg 'FormGroup<\{' src/app --count=5
# Check if there are any form type definitions/interfaces
rg 'interface.*Form|type.*Form.*=' src/app/system/external-services --count=10
# Look at the entire component to see if there's any type definition
cat src/app/system/external-services/ollama/edit-ollama/edit-ollama.component.tsRepository: openMF/web-app
Length of output: 4357
🏁 Script executed:
#!/bin/bash
# Check how many components use UntypedFormBuilder vs typed forms
echo "=== UntypedFormBuilder usage ==="
rg 'UntypedFormBuilder' src/app --count
echo "=== Typed FormBuilder usage ==="
rg 'FormBuilder.*<\{|FormGroup<\{' src/app --count
# Check if there's a pattern for form type definitions in the codebase
rg -B 2 'FormGroup<' src/app | head -30Repository: openMF/web-app
Length of output: 21546
Use typed reactive forms instead of untyped.
The component imports UntypedFormBuilder and UntypedFormGroup (lines 10, 36, 45), which drops type safety for form controls and validators. Since this is a new component and Angular 20 fully supports typed forms, define a form type and use FormGroup<{ enabled: FormControl<boolean>; url: FormControl<string>; model: FormControl<string> }> to align with strict typing conventions required by the coding guidelines.
🤖 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/edit-ollama/edit-ollama.component.ts`
at 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.
Source: Coding guidelines
| this.ollamaService.checkConnectionAt(url).subscribe((ok) => { | ||
| if (ok) { | ||
| this.alertService.alert({ | ||
| type: 'Ollama', | ||
| message: this.translateService.instant('labels.text.Ollama connection successful') | ||
| }); | ||
| this.loadModels(url); | ||
| } else { | ||
| this.alertService.alert({ | ||
| type: 'Ollama Error', | ||
| message: this.translateService.instant('labels.text.Ollama connection failed') | ||
| }); | ||
| } |
There was a problem hiding this comment.
testConnection() currently reports success for unreachable non-empty URLs.
Line 93 depends on checkConnectionAt(url), but the current service contract maps to models.length >= 0 (see src/app/shared/services/ollama.service.ts:118-127), which is always true for arrays. That makes the failure branch at Lines 100-105 effectively dead for any non-empty URL and can show false success alerts.
🤖 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/edit-ollama/edit-ollama.component.ts`
around lines 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.
| 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
| const rows = [ | ||
| { name: 'enabled', value: this.settingsService.ollamaEnabled ? 'true' : 'false' }, | ||
| { name: 'url', value: this.settingsService.ollamaUrl || '—' }, | ||
| { name: 'model', value: this.settingsService.ollamaModel || '—' } |
There was a problem hiding this comment.
Localize the enabled-state display value.
Line 70 stores 'true'/'false' as literals and the template renders them directly, so this value won’t localize in non-English UIs.
As per coding guidelines, src/app/**/*.{html,ts} should use i18n variables for all user-facing strings instead of hardcoded text.
🤖 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.ts` around lines 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.
Source: Coding guidelines
| font-size: 0.85rem; | ||
| line-height: 1.6; | ||
| white-space: pre-wrap; | ||
| word-break: break-word; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
word-break break-word deprecated CSS overflow-wrap modern replacement
💡 Result:
The CSS value word-break: break-word is officially deprecated [1][2][3]. It was originally supported by browsers as a legacy alias for overflow-wrap: break-word (or sometimes behave like overflow-wrap: anywhere), and it is not a formally defined value in the standard CSS Text Module specifications [4][5]. Modern Replacement The recommended replacement for word-break: break-word is overflow-wrap: anywhere [1][5]. Why use overflow-wrap: anywhere? 1. Standardization: It is the standard-compliant way to allow browsers to break otherwise unbreakable strings (like long URLs or hashes) at an arbitrary point if they would otherwise overflow their container [3][4]. 2. Intrinsic Sizing: Unlike some legacy implementations, overflow-wrap: anywhere correctly accounts for line-break opportunities when calculating the min-content size of elements, ensuring that containers like inline-blocks or flex items do not expand excessively due to long, unbroken strings [4][5][6]. Summary of Recommendations * For general text overflow prevention: Use overflow-wrap: anywhere [5]. * If you need to manage line breaking in non-English (e.g., CJK) content: Use the word-break property with values like normal (the default) or keep-all [5][7]. * Legacy support: While you should avoid using word-break: break-word in new code, it is preserved in browser engines for backward compatibility with older stylesheets [8][3][4]. Context on Property Roles * overflow-wrap: Designed to handle overflow by allowing breaks in long words when no other wrap opportunities exist [9][7]. * word-break: Designed to control line-breaking rules, particularly for languages that do not use spaces as word separators (like CJK languages) [3][7]. Using it to force line breaks in long strings is generally considered a misuse of the property's primary purpose [5][7].
Citations:
- 1: https://web-platform-dx.github.io/web-features-explorer/features/word-break-break-word/
- 2: https://github.com/mdn/content/blob/main/files/en-us/web/css/reference/properties/word-break/index.md
- 3: https://www.w3.org/TR/css-text-4/
- 4: https://www.w3.org/TR/css-text/
- 5: https://deku.posstree.com/en/web/css-overflow-wrap/
- 6: https://stackoverflow.com/questions/66724509/overflow-wrap-break-word-vs-word-break-break-word
- 7: https://css-tricks.com/almanac/properties/o/overflow-wrap/
- 8: https://stackoverflow.com/questions/17259916/difference-between-overflow-wrap-and-word-break
- 9: https://developer.mozilla.org/en-US/docs/Web/CSS/Guides/Text/Wrapping_breaking_text
🏁 Script executed:
# Verify the file and exact content at the specified location
cd src/app/system/manage-reports/create-report && cat -n create-report.component.scss | sed -n '50,65p'Repository: openMF/web-app
Length of output: 428
Replace deprecated word-break: break-word.
Use overflow-wrap: anywhere; to properly handle long unbreakable strings while adhering to modern CSS standards.
Suggested fix
.ai-sql-preview {
@@
- word-break: break-word;
+ overflow-wrap: anywhere;
@@
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| word-break: break-word; | |
| .ai-sql-preview { | |
| overflow-wrap: anywhere; | |
| } |
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 57-57: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🤖 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` at
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.
Source: Linters/SAST tools
IOhacker
left a comment
IOhacker
left a comment
There was a problem hiding this comment.
Hello, there is an ongoing effort for implementing a Mifos copilot features. @shubhamkumar9199 could you please review
| * real table and column names. | ||
| */ | ||
| generateSqlQuery(description: string): Observable<string> { | ||
| const system = `You are a Mifos Fineract MySQL report query generator. |
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 | |||
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> | |||
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
2 participants
Description
Added ollama to the external services, allowing admins to use it to generate reports.
Summary by CodeRabbit
Release Notes
New Features
Style