docs: add AI engineering guidance#3691
Conversation
|
Warning Review limit reached
More reviews will be available in 50 minutes and 29 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
Note
|
| Layer / File(s) | Summary |
|---|---|
AI engineering guidance document docs/AI_ENGINEERING_GUIDANCE.md |
Creates a guidance page with guardrails, phased roadmap, API/UI rules, checklists, and a suggested GitHub issue. |
Project Scope Documentation
| Layer / File(s) | Summary |
|---|---|
Project scope document docs/PROJECT_SCOPE.md |
Creates a scope document covering product identity, principles, in-scope and out-of-scope areas, a relationship diagram, and success criteria. |
Stylelint Configuration
| Layer / File(s) | Summary |
|---|---|
Stylelint config update .stylelintrc.json |
Adds standard and recommended SCSS Stylelint extensions, registers the SCSS plugin, disables selector-class-pattern, and enables scss/at-rule-no-unknown. |
Estimated code review effort
🎯 1 (Trivial) | ⏱️ ~5 minutes
🚥 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 clearly describes the main change: adding AI engineering guidance documentation. |
| 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.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/AI_ENGINEERING_GUIDANCE.md (2)
5-5: 📐 Maintainability & Code Quality | 🔵 TrivialConsider parameterizing repository reference.
The scope line hardcodes
nandahtoon/Mifosx-web-app. If this guidance is ever forked or the repository renamed, this becomes stale. Consider using a more generic reference or noting that this should be updated if moved.🤖 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 `@docs/AI_ENGINEERING_GUIDANCE.md` at line 5, The scope line hardcodes the repository name, which can become stale if the guidance is forked or renamed. Update the opening scope text in AI_ENGINEERING_GUIDANCE.md to use a generic repository reference or a note that it should be changed when moved, so the document stays portable without tying it to nandahtoon/Mifosx-web-app.
89-102: 📐 Maintainability & Code Quality | 🔵 TrivialConsider noting Angular standalone component approach.
The suggested
dashboard.module.tsanddashboard-routing.module.tsimply NgModules. If the project is on Angular 20, standalone components are the modern default. Consider adding guidance on whether to:
- Stay consistent with existing NgModule patterns in this codebase, or
- Adopt standalone components for new features.
This affects the suggested structure and long-term maintainability.
🤖 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 `@docs/AI_ENGINEERING_GUIDANCE.md` around lines 89 - 102, The dashboard structure guidance currently implies NgModule-based files like dashboard.module.ts and dashboard-routing.module.ts, which may conflict with Angular 20’s standalone default. Update the guidance to explicitly state whether new dashboard features should follow the existing NgModule pattern or use standalone components, and align the listed artifacts accordingly. Reference the dashboard component area and routing/module naming so readers know whether to create dashboard.component.ts with standalone imports or keep module-based organization.
🤖 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 `@docs/AI_ENGINEERING_GUIDANCE.md`:
- Line 54: The CI fallback in the build workflow regenerates the dependency
graph by deleting package-lock.json and rerunning npm install after npm ci
fails, which should be removed. Update the workflow logic in the build job so it
fails fast on npm ci errors instead of retrying with a regenerated lockfile, and
keep the existing lockfile as the source of truth. Locate the fallback around
the build workflow’s npm ci handling and remove the delete-and-reinstall branch.
---
Nitpick comments:
In `@docs/AI_ENGINEERING_GUIDANCE.md`:
- Line 5: The scope line hardcodes the repository name, which can become stale
if the guidance is forked or renamed. Update the opening scope text in
AI_ENGINEERING_GUIDANCE.md to use a generic repository reference or a note that
it should be changed when moved, so the document stays portable without tying it
to nandahtoon/Mifosx-web-app.
- Around line 89-102: The dashboard structure guidance currently implies
NgModule-based files like dashboard.module.ts and dashboard-routing.module.ts,
which may conflict with Angular 20’s standalone default. Update the guidance to
explicitly state whether new dashboard features should follow the existing
NgModule pattern or use standalone components, and align the listed artifacts
accordingly. Reference the dashboard component area and routing/module naming so
readers know whether to create dashboard.component.ts with standalone imports or
keep module-based organization.
🪄 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: 06d2165c-255d-49d0-972e-d1911f236700
📒 Files selected for processing (1)
docs/AI_ENGINEERING_GUIDANCE.md
| - Run `npm run lint`. | ||
| - Run `npm run test:ci`. | ||
| - Run `npm run build:prod`. | ||
| - Remove CI fallback that deletes `package-lock.json` and runs `npm install`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for CI configurations that delete package-lock or use npm install fallback
fd -i -e yml -e yaml .github/ | xargs grep -l 'package-lock\|npm install' 2>/dev/null
rg -n 'rm.*package-lock|npm install' .github/ --type yaml 2>/dev/null | head -20Repository: openMF/web-app
Length of output: 622
🏁 Script executed:
#!/bin/bash
sed -n '30,48p' .github/workflows/build.ymlRepository: openMF/web-app
Length of output: 620
Remove the lockfile-regenerating CI fallback
.github/workflows/build.yml deletes package-lock.json and reruns npm install on npm ci failure, which can let CI pass against a different dependency graph than the committed lockfile. Surface the failure instead of regenerating the lockfile here.
🤖 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 `@docs/AI_ENGINEERING_GUIDANCE.md` at line 54, The CI fallback in the build
workflow regenerates the dependency graph by deleting package-lock.json and
rerunning npm install after npm ci fails, which should be removed. Update the
workflow logic in the build job so it fails fast on npm ci errors instead of
retrying with a regenerated lockfile, and keep the existing lockfile as the
source of truth. Locate the fallback around the build workflow’s npm ci handling
and remove the delete-and-reinstall branch.
IOhacker
left a comment
There was a problem hiding this comment.
@nandahtoon could you please add a video how it is being used?
Description
Describe the changes made and why they were made instead of how they were made. List any dependencies that are required for this change.
Related issues and discussion
#{Issue Number}
Screenshots, if any
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit