Skip to content

fix: resolve 3 bugs — cacheHitRate, null depth, checkpoint TypeError - #581

Merged
Pedrovaleriolopez merged 1 commit into
SynkraAI:mainfrom
rafaelscosta:fix/triple-bugfix-499-512-513
Mar 11, 2026
Merged

fix: resolve 3 bugs — cacheHitRate, null depth, checkpoint TypeError#581
Pedrovaleriolopez merged 1 commit into
SynkraAI:mainfrom
rafaelscosta:fix/triple-bugfix-499-512-513

Conversation

@rafaelscosta

@rafaelscosta rafaelscosta commented Mar 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes 3 reported bugs in a single PR:

Files changed

File Bug Change
.aiox-core/core/config/config-loader.js #499 Denominator: loadscacheHits + cacheMisses
.aiox-core/core/utils/yaml-validator.js #512 Added && value !== null in getMaxDepth loop
.aiox-core/core/execution/build-state-manager.js #513 Added `!this._state.checkpoints

Test plan

  • build-state-manager.test.js — 49/49 passing
  • Verify cacheHitRate no longer exceeds 100% with mixed cache hits/misses
  • Verify YAML files with null values don't inflate depth count

Closes #499, closes #512, closes #513

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved cache metrics calculation to better reflect actual cache activity
    • Enhanced robustness when retrieving checkpoint data
    • Fixed depth calculation to properly exclude null values in nested object validation
  • Chores

    • Updated manifest file with current metadata and timestamps

…oint TypeError

- config-loader: use (cacheHits + cacheMisses) as denominator for cacheHitRate
  instead of loads (disk-only), preventing rates >100% (fixes SynkraAI#499)
- yaml-validator: add null guard in getMaxDepth inner loop to prevent
  null values from inflating depth count (fixes SynkraAI#512)
- build-state-manager: guard against undefined checkpoints array in
  getLastCheckpoint to prevent TypeError (fixes SynkraAI#513)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel

vercel Bot commented Mar 10, 2026

Copy link
Copy Markdown

@rafaelscosta 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.

@coderabbitai

coderabbitai Bot commented Mar 10, 2026

Copy link
Copy Markdown

Walkthrough

This PR fixes three distinct bugs: correcting cache hit rate calculation to use actual cache interactions as denominator, adding a null guard to prevent TypeError when retrieving checkpoints, and excluding null values from YAML depth calculations. The manifest metadata is also updated.

Changes

Cohort / File(s) Summary
Bug Fixes
core/config/config-loader.js, core/execution/build-state-manager.js, core/utils/yaml-validator.js
Three targeted robustness improvements: (1) cacheHitRate now correctly divides by cacheHits + cacheMisses instead of loads; (2) getLastCheckpoint guards against undefined checkpoints array before accessing length; (3) getMaxDepth explicitly checks that value is not null before recursing into nested objects.
Manifest Update
install-manifest.yaml
Timestamp updated and file metadata (hashes and sizes) refreshed across all tracked paths. No structural changes to manifest.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes manifest file updates (hashes and timestamps) that are out-of-scope artifacts unrelated to the bug fixes, though the actual code fixes remain on-target. Regenerate the install-manifest.yaml file after the code changes are finalized to ensure it reflects only necessary updates, or exclude manifest changes from this PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the three bugs being fixed: cacheHitRate denominator correction, null depth handling, and checkpoint TypeError prevention.
Linked Issues check ✅ Passed All three linked issues (#499, #512, #513) are directly addressed with corresponding code changes that implement the specified fixes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.aiox-core/core/utils/yaml-validator.js (1)

257-266: ⚠️ Potential issue | 🟠 Major

Guard getMaxDepth() against cyclic aliases, not just null.

validateGeneral() records circular YAML via JSON.stringify, but it still calls getMaxDepth() afterward. A self-referential alias will still recurse forever here and turn a validation result into a RangeError.

Proposed fix
-  getMaxDepth(obj, currentDepth = 0) {
+  getMaxDepth(obj, currentDepth = 0, ancestors = new WeakSet()) {
     if (typeof obj !== 'object' || obj === null) {
       return currentDepth;
     }
+    if (ancestors.has(obj)) {
+      return currentDepth;
+    }
+    ancestors.add(obj);
 
     let maxDepth = currentDepth;
     for (const value of Object.values(obj)) {
       if (typeof value === 'object' && value !== null) {
-        const depth = this.getMaxDepth(value, currentDepth + 1);
+        const depth = this.getMaxDepth(value, currentDepth + 1, ancestors);
         maxDepth = Math.max(maxDepth, depth);
       }
     }
+    ancestors.delete(obj);
 
     return maxDepth;
   }

As per coding guidelines, Verify error handling is comprehensive with proper try/catch and error context.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.aiox-core/core/utils/yaml-validator.js around lines 257 - 266, The
getMaxDepth(obj, currentDepth = 0) helper must guard against
cyclic/self-referential YAML aliases (not only null), so modify getMaxDepth to
accept or create a visited set/WeakSet and skip traversing any object already
seen to avoid infinite recursion; update calls from validateGeneral (which
serializes circular refs) to pass a new visited container when kicking off the
recursion. Specifically, in getMaxDepth ensure you check visited.has(obj) before
recursing, add obj to visited when first encountered, and use a WeakSet to avoid
memory leaks so circular structures return a finite depth instead of causing a
RangeError.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In @.aiox-core/core/utils/yaml-validator.js:
- Around line 257-266: The getMaxDepth(obj, currentDepth = 0) helper must guard
against cyclic/self-referential YAML aliases (not only null), so modify
getMaxDepth to accept or create a visited set/WeakSet and skip traversing any
object already seen to avoid infinite recursion; update calls from
validateGeneral (which serializes circular refs) to pass a new visited container
when kicking off the recursion. Specifically, in getMaxDepth ensure you check
visited.has(obj) before recursing, add obj to visited when first encountered,
and use a WeakSet to avoid memory leaks so circular structures return a finite
depth instead of causing a RangeError.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d3c6f7b4-ac0d-47b2-897c-f086fb55bfda

📥 Commits

Reviewing files that changed from the base of the PR and between ff711c1 and cee5125.

📒 Files selected for processing (4)
  • .aiox-core/core/config/config-loader.js
  • .aiox-core/core/execution/build-state-manager.js
  • .aiox-core/core/utils/yaml-validator.js
  • .aiox-core/install-manifest.yaml

@oalanicolas

Copy link
Copy Markdown
Collaborator

🔍 Maintainer Review — @devops (Gage)

Good work on fixing 3 bugs (#499, #512, #513). The fixes are clean and correct:

  • cacheHitRate denominator fix in config-loader.js
  • null guard in yaml-validator.js getMaxDepth()
  • checkpoints null check in build-state-manager.js

However, this PR does NOT fix #497 (ConfigCache.has() inflating stats). The has() method in both core/config/config-cache.js and infrastructure/scripts/config-cache.js still delegates to get(), which increments hits/misses as a side-effect.

We'll address #497 in a separate dedicated fix branch.

CodeRabbit: ✅ APPROVED
CI: ✅ Welcome passed

@nikolasdehor

Copy link
Copy Markdown
Contributor

Atenção @Pedrovaleriolopez @oalanicolas — esse PR aborda exatamente os mesmos 3 bugs que já temos PRs abertos há mais de 2 semanas:

Os 3 PRs originais já estavam com testes, rebased em main e prontos pra merge. Seria justo dar prioridade aos PRs que chegaram primeiro, não?

@Pedrovaleriolopez
Pedrovaleriolopez merged commit 6f27bc1 into SynkraAI:main Mar 11, 2026
2 of 3 checks passed
backstagegrow pushed a commit to backstagegrow/aios-core that referenced this pull request Apr 6, 2026
…oint TypeError (SynkraAI#581)

Cherry-picked from SynkraAI/aiox-core 6f27bc1 — keeping aios-core identity.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
brunajunckes pushed a commit to brunajunckes/aiox-core that referenced this pull request Apr 8, 2026
…oint TypeError (SynkraAI#581)

fix: resolve 3 bugs — cacheHitRate denominator, null depth guard, checkpoint TypeError
brunajunckes pushed a commit to brunajunckes/aiox-core that referenced this pull request Apr 11, 2026
…oint TypeError (SynkraAI#581)

fix: resolve 3 bugs — cacheHitRate denominator, null depth guard, checkpoint TypeError
brunajunckes pushed a commit to brunajunckes/aiox-core that referenced this pull request Apr 16, 2026
…oint TypeError (SynkraAI#581)

fix: resolve 3 bugs — cacheHitRate denominator, null depth guard, checkpoint TypeError
brunajunckes pushed a commit to brunajunckes/aiox-core that referenced this pull request Apr 16, 2026
…oint TypeError (SynkraAI#581)

fix: resolve 3 bugs — cacheHitRate denominator, null depth guard, checkpoint TypeError
alexandreyuan pushed a commit to alexandreyuan/aios-core that referenced this pull request May 13, 2026
- Added project-status task for accurate *status command (SynkraAI#559)
- Fixed ConfigCache.has() stat inflation and hook system errors (SynkraAI#582)
- Fixed CI: use pull_request_target for fork PR labeling (SynkraAI#585)
- Fixed cacheHitRate overflow, null depth count, checkpoint TypeError (SynkraAI#581)
- Rebrand to AIOX Squad (SynkraAI#553)

Regenerated install-manifest.yaml (1090 files, sha256:ea053983d78f5ef6).
@alexandreyuan alexandreyuan mentioned this pull request May 13, 2026
3 tasks
tuanmedeiros pushed a commit to tuanmedeiros/aios-core-synkraay that referenced this pull request Jun 2, 2026
…oint TypeError (SynkraAI#581)

fix: resolve 3 bugs — cacheHitRate denominator, null depth guard, checkpoint TypeError
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants