Skip to content

Conversation

@marcelorodrigo
Copy link
Member

@marcelorodrigo marcelorodrigo commented Oct 1, 2025

Summary by CodeRabbit

  • New Features

    • None
  • Bug Fixes

    • Fixed data source path for indicators so index updates are detected and applied reliably.
  • Chores

    • Upgraded framework to Nuxt 4 and added Pinia dependency; enabled future compatibility flag.
    • Refined automation workflow to reliably detect, commit, and push index updates.
  • Tests

    • Updated test imports for new project layout and added comprehensive CDB calculation tests.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

Updated file paths for indicadores.json in CI workflow and script, adjusted a test import and added a new CDB test, added Nuxt future compatibilityVersion: 4, and upgraded Nuxt/Pinia dependencies in package.json. No behavioral control-flow changes beyond path locations and config version bump.

Changes

Cohort / File(s) Summary
Indicadores path updates
.github/workflows/update-indexes.yml, update-indexes.js
Change references from assets/indicadores.json to app/assets/indicadores.json in workflow detection and the update script; workflow logic unchanged.
Framework upgrade & config
package.json, nuxt.config.ts
Bump Nuxt to 4.x, add pinia v3 dependency, upgrade @pinia/nuxt to 0.11.x; add future: { compatibilityVersion: 4 } to Nuxt config.
Test changes
test/app/src/finance.spec.ts, test/app/src/cdb.spec.ts
Update import path in finance.spec.ts to ../../../app/src/finance; add new cdb.spec.ts with tests for getCDBResult, IOF, and tax brackets.

Sequence Diagram(s)

sequenceDiagram
  actor DevOps
  participant GH_Actions as "GitHub Actions\nupdate-indexes.yml"
  participant Script as "update-indexes.js"
  participant Repo as "Repository (app/assets/indicadores.json)"

  DevOps->>GH_Actions: push commit
  GH_Actions->>Script: run change-detection (checks app/assets/indicadores.json)
  Script->>Repo: read app/assets/indicadores.json
  Script-->>GH_Actions: return has_changes (true/false)
  alt has_changes == true
    GH_Actions->>Repo: git add/commit/push (app/assets/indicadores.json)
    GH_Actions->>GH_Actions: trigger follow-up workflow
  else has_changes == false
    GH_Actions-->>DevOps: no changes detected
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

I nibble bytes beneath moonlit commits,
Paths hop to new gardens where JSON sits,
Nuxt grows to four, Pinia springs near,
Tests hum softly — the output is clear. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title “Nuxt 4” is related to the core change of moving to Nuxt version 4 but is overly terse and does not clearly describe the update action or context, making it difficult for reviewers scanning history to understand that this pull request upgrades the project to Nuxt 4. Consider renaming the title to something more descriptive such as “Upgrade project to Nuxt 4” or “Migrate application to Nuxt 4” to clearly convey the primary change and intent.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch nuxt-4

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd40e49 and 24c867f.

📒 Files selected for processing (1)
  • test/app/src/cdb.spec.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/app/src/cdb.spec.ts (2)
app/src/finance.ts (3)
  • getIndexIR (6-16)
  • getIOFAmount (32-35)
  • getIOFPercentage (18-30)
app/src/cdb.ts (1)
  • getCDBResult (3-18)
🔇 Additional comments (5)
test/app/src/cdb.spec.ts (5)

1-3: LGTM! Imports are correct.

All necessary testing utilities and financial functions are properly imported with correct relative paths.


17-31: LGTM! Tax tier tests cover all brackets correctly.

The test cases properly validate all four tax percentage tiers (22.5%, 20%, 17.5%, 15%) based on the investment duration, matching the getIndexIR implementation.


33-67: LGTM! IOF impact tests are thorough.

Both test suites correctly validate:

  • IOF application within 30 days (tested at 10 days)
  • No IOF after 30 days (tested at 200 days)

The use of toBeCloseTo for floating-point comparisons is appropriate.


69-84: LGTM! Long-term tax bracket test is correct.

The test properly validates the 15% tax rate for investments exceeding 720 days and confirms no IOF is applied for long-term investments.


5-15: Ignore rounding concern – compoundInterest already applies two-decimal rounding (parseFloat(interest.toFixed(2))), matching the test helper.

Likely an incorrect or invalid review comment.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 700aaec and bd40e49.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • .github/workflows/update-indexes.yml (2 hunks)
  • nuxt.config.ts (1 hunks)
  • package.json (1 hunks)
  • test/app/src/finance.spec.ts (1 hunks)
  • update-indexes.js (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/app/src/finance.spec.ts (1)
test/src/finance.spec.ts (6)
  • it (48-70)
  • expect (27-30)
  • interestAmount (49-58)
  • testCases (4-23)
  • interestAmount (60-69)
  • it (26-31)
🔇 Additional comments (4)
update-indexes.js (1)

5-5: LGTM! Path updated consistently.

The path change from assets/indicadores.json to app/assets/indicadores.json aligns with the Nuxt 4 directory restructuring and is used consistently for both reading (line 6) and writing (line 65).

.github/workflows/update-indexes.yml (1)

30-30: LGTM! Workflow paths updated consistently.

Both the change detection (line 30) and git add (line 40) commands correctly reference the new app/assets/indicadores.json path, which aligns with the path change in update-indexes.js.

Also applies to: 40-40

test/app/src/finance.spec.ts (1)

2-2: Finance module verified at new location. Confirmed that app/src/finance.ts exists and matches the test import path; approving changes.

package.json (1)

25-28: Approve Pinia integration
Pinia v3.0.3 and @pinia/nuxt v0.11.2 are officially supported in Nuxt 4 per the Nuxt 4 migration guide (nuxt.com). GitHub’s securityVulnerabilities API returned no advisories for either package.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 1, 2025

@marcelorodrigo marcelorodrigo merged commit 6aa7226 into master Oct 1, 2025
4 checks passed
@marcelorodrigo marcelorodrigo deleted the nuxt-4 branch October 1, 2025 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants