Capture row view by value in async formatter

[dajiaohuang]

Summary

• Capture the row_view by value before scheduling row_view_to_string on the main queue.
• Keep the shared dataset view alive if the caller's reference or temporary expires before execution.

Closes #3235.

Validation: source review and git diff --check only. No build or tests were run.

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability when converting rows to text during asynchronous operations.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 2d0d3eb4-71f3-493e-b327-4bb323fb0c00

📥 Commits

Reviewing files that changed from the base of the PR and between f432041 and 011429d.

📒 Files selected for processing (1)

• cpp/deeplake_api/row_view.hpp

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 1 remain after this review.

---

📝 Walkthrough

Walkthrough

row_view_to_string now captures its row_view argument by value in the lambda passed to async::run_on_main. The function signature and call to to_string() are unchanged.

Changes

Row view conversion

Layer / File(s)	Summary
Capture row view by value cpp/deeplake_api/row_view.hpp	The lambda passed to async::run_on_main captures the row view by value instead of by reference. It still calls to_string() on the captured row view.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 01142

The change preserves the row view during asynchronous conversion. No merge-blocking issue was identified; run the usual build and tests before merging.

Architecture Summary

Architecture risk: 🔵 Low · up to 01142

The change affects 1 system.

Changed systems: cpp

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — cpp (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in cpp/deeplake_api/row_view.hpp: row_view_to_string changes the async::run_on_main lambda capture from a reference to a value; it still calls to_string() on the captured row view.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the fix, its lifetime-safety purpose, the linked issue, and validation performed. However, it omits the required Impact, Description, Things to be aware of, Things to worry ab…	Update the description to use the repository template. Mark the applicable Impact checkbox, add a Description section with the problem and user impact, document the technical choices in Things to be aware of, state any concerns or unknowns …
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: capturing the row view by value in the asynchronous formatter.
Linked Issues check	✅ Passed	The PR addresses the coding requirement in issue #3235. In cpp/deeplake_api/row_view.hpp, row_view_to_string(const row_view& r) changes the queued lambda capture from reference to value. The lambd…
Out of Scope Changes check	✅ Passed	The reported PR change is limited to the lambda capture in cpp/deeplake_api/row_view.hpp. This change directly implements issue #3235 by preventing a queued lambda from using a dangling row_view r…

Full details: Description check

Explanation

The description explains the fix, its lifetime-safety purpose, the linked issue, and validation performed. However, it omits the required Impact, Description, Things to be aware of, Things to worry about, and Additional Context sections from the repository template.

Resolution

Update the description to use the repository template. Mark the applicable Impact checkbox, add a Description section with the problem and user impact, document the technical choices in Things to be aware of, state any concerns or unknowns in Things to worry about, and add relevant context. Retain the issue reference and validation details.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

_{A rabbit checks the queued-up call,
And keeps the row view safe through all.
By-value ears hold tight and stay,
While to_string() finds its way.
Then hops along to nibble hay.}

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

[khustup2]

No live defect here: both callers hold the row view for the duration of the blocking call, so the by-reference capture never dangles. The change is harmless but adds nothing; holding.