Skip to content

fix(config-loader): cacheHitRate no longer exceeds 100%#500

Open
nikolasdehor wants to merge 1 commit intoSynkraAI:mainfrom
nikolasdehor:fix/config-loader-hit-rate-formula
Open

fix(config-loader): cacheHitRate no longer exceeds 100%#500
nikolasdehor wants to merge 1 commit intoSynkraAI:mainfrom
nikolasdehor:fix/config-loader-hit-rate-formula

Conversation

@nikolasdehor
Copy link
Contributor

@nikolasdehor nikolasdehor commented Feb 24, 2026

Resumo

  • Corrige fórmula cacheHitRate em getPerformanceMetrics() que podia ultrapassar 100%
  • Antes: hits / loads — como loads conta apenas leituras do disco e hits conta acessos ao cache, a taxa podia exceder 100%
  • Depois: hits / (hits + misses) — denominador correto que representa o total de consultas ao cache

Closes #499

Plano de teste

  • 25 testes unitários passam localmente
  • Inclui 2 testes de regressão para verificar que a taxa nunca excede 100%

The old formula divided cacheHits by loads (disk reads only), which
could produce rates above 100% since most requests are cache hits that
never touch disk.

Use (cacheHits + cacheMisses) as the denominator for a correct
percentage.

25 unit tests including 2 regression tests for this fix.

Closes SynkraAI#499
Copilot AI review requested due to automatic review settings February 24, 2026 20:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where the cache hit rate calculation in config-loader.js could exceed 100%. The issue occurred because the denominator used only disk loads (cache misses) instead of total requests.

Changes:

  • Updated getPerformanceMetrics() to calculate cache hit rate as cacheHits / (cacheHits + cacheMisses) × 100
  • Added comprehensive unit tests (25 total) for the config-loader module
  • Included 2 regression tests specifically validating the fix for issue #499

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/core/config/config-loader.test.js New test suite with 25 tests covering all config-loader functions and regression tests for bug #499
.aios-core/install-manifest.yaml Updated manifest with new file hashes, sizes, and generation timestamp
.aios-core/core/config/config-loader.js Fixed cache hit rate formula to use total requests as denominator

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2026

Walkthrough

This PR addresses issue #499 by fixing the cache hit rate calculation in config-loader.js to use total cache requests (cacheHits + cacheMisses) as the denominator instead of disk loads, preventing rates exceeding 100%. Comprehensive test coverage is added, and manifest metadata is updated to reflect content changes.

Changes

Cohort / File(s) Summary
Cache Performance Metric Fix
.aios-core/core/config/config-loader.js
Reworks cacheHitRate calculation in getPerformanceMetrics() to divide by (cacheHits + cacheMisses) instead of loads. Returns percentage bounded by [0, 100], or '0%' when no cache requests exist.
Test Coverage
tests/core/config/config-loader.test.js
Adds 378 lines of comprehensive unit tests covering exported constants, config loading behavior, cache mechanics, performance metrics, agent configuration validation, and regression test for cacheHitRate formula fix.
Manifest Updates
.aios-core/install-manifest.yaml
Updates generated_at timestamp and file entry metadata (hash and size values) across multiple tracked files in core, data, development, infrastructure, and product sections.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

core, tests, installer

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main fix: correcting the cacheHitRate formula to prevent exceeding 100%.
Linked Issues check ✅ Passed The pull request fully addresses issue #499 by modifying config-loader.js to calculate cacheHitRate using (cacheHits + cacheMisses) as denominator and includes comprehensive regression tests.
Out of Scope Changes check ✅ Passed All changes directly support the cacheHitRate fix: the formula correction, supporting manifest update, and comprehensive test suite with regression tests for issue #499.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a 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.

@vercel
Copy link

vercel bot commented Feb 24, 2026

@nikolasdehor is attempting to deploy a commit to the Pedro Valério Lopez's projects Team on Vercel.

A member of the Team first needs to authorize it.

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.

bug: getPerformanceMetrics() cacheHitRate divides by disk loads instead of total requests

2 participants