Fix: handle undefined binding.size to prevent NaN -fixes #262

[shriya-gatate]

##Problem
When binding.size is undefined, totalSize becomes NaN, showing wrong file count and size in the UI.

##Fix
Added a null check to default to 0 if binding.size is undefined.

Fixes #262

Summary by CodeRabbit

• Bug Fixes
  • Improved file browser size calculations so totals no longer break when query results are missing size data.
  • File sizes now default to zero when unavailable, making totals more reliable and preventing incorrect accumulation.

[coderabbitai]

Warning

Review limit reached

@shriya-gatate, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 10 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 reviews.

How do review 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 refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b3185286-e6f2-43e7-b3f7-f5ecab1d6888

📥 Commits

Reviewing files that changed from the base of the PR and between d606ef1 and 6cdf54a.

📒 Files selected for processing (1)

• public/js/components/file-browser/file-browser.js

📝 Walkthrough

Walkthrough

file-browser.js now guards binding.size in querySparql both when parsing numericalValue and when accumulating ctrl.totalSize, defaulting missing size data to 0.

Changes

File Browser Size Guard

Layer / File(s)	Summary
Conditional size parsing and accumulation public/js/components/file-browser/file-browser.js	querySparql now sets binding.size.numericalValue only when binding.size exists and adds it to ctrl.totalSize with a 0 fallback when it does not.

Estimated code review effort: 1 (Trivial) | ~2 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 fix for undefined binding.size and references the related issue.
Linked Issues check	✅ Passed	The change adds a null check for binding.size, preventing NaN totals and matching issue #262.
Out of Scope Changes check	✅ Passed	The diff is narrowly scoped to the SPARQL result handling fix and adds no unrelated behavior.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@public/js/components/file-browser/file-browser.js`:
- Line 145: The null fallback in file-browser.js only protects the totalSize
addition, but binding.size is still dereferenced earlier in the same flow.
Update the size parsing logic in the file-browser binding handling so the
null/undefined check happens before accessing binding.size.value or
binding.size.numericalValue, store the parsed size once, and reuse that parsed
value when updating ctrl.totalSize.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c2e1f91e-596d-4286-aa9b-8dd99213f55b

📥 Commits

Reviewing files that changed from the base of the PR and between 2c775e7 and 0d00b8e.

📒 Files selected for processing (1)

• public/js/components/file-browser/file-browser.js

[coderabbitai]

♻️ Duplicate comments (1)

public/js/components/file-browser/file-browser.js (1)

135-135: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Guard still doesn't prevent the crash on missing binding.size.

The ternary at Line 135 only decides the value assigned; the assignment target binding.size.numericalValue = ... still unconditionally dereferences binding.size. When binding.size is undefined, this throws TypeError: Cannot set properties of undefined, which is swallowed by the catch block in querySparql, aborting the loop before Line 145's fallback ever runs. This is the same root cause raised in the previous review round — the fix needs to guard the property write itself, not just the computed value.

🐛 Proposed fix

       for (var b in ctrl.queryResult.bindings) {
         var binding = ctrl.queryResult.bindings[b];
-        binding.size.numericalValue = binding.size? parseInt(binding.size.value):0;
+        var numericalSize = binding.size ? parseInt(binding.size.value, 10) : 0;
+        if (binding.size) {
+          binding.size.numericalValue = numericalSize;
+        }
         ctrl.queryResult.uriList += binding.file.value + "\n";

         if (binding.variant != undefined) {
           binding.variant.value = ctrl.formatVariant(binding.variant.value);
         }

-        ctrl.totalSize += binding.size?binding.size.numericalValue:0;
+        ctrl.totalSize += numericalSize;
         ctrl.numFiles++;
       }

Also applies to: 145-145

🤖 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 `@public/js/components/file-browser/file-browser.js` at line 135, The
`querySparql` loop in `file-browser.js` still crashes when `binding.size` is
missing because the assignment to `binding.size.numericalValue` dereferences
`binding.size` before the ternary runs. Update the size-handling logic around
`binding.size` so the property write itself is guarded, and only set
`numericalValue` when `binding.size` exists; otherwise leave it untouched so the
fallback path later in the same loop can execute.

🤖 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.

Duplicate comments:
In `@public/js/components/file-browser/file-browser.js`:
- Line 135: The `querySparql` loop in `file-browser.js` still crashes when
`binding.size` is missing because the assignment to
`binding.size.numericalValue` dereferences `binding.size` before the ternary
runs. Update the size-handling logic around `binding.size` so the property write
itself is guarded, and only set `numericalValue` when `binding.size` exists;
otherwise leave it untouched so the fallback path later in the same loop can
execute.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 48877964-861b-4b22-a023-e85709757658

📥 Commits

Reviewing files that changed from the base of the PR and between 0d00b8e and d606ef1.

📒 Files selected for processing (1)

• public/js/components/file-browser/file-browser.js

[shriya-gatate]

Addressed CodeRabbit's feedback — moved the null check to a local variable
(sizeValue) to avoid dereferencing binding.size before the guard runs.
Line 145 now reuses sizeValue directly.