test: add InvestmentRollup helper for testing automation loop

[ak684]

No description provided.

[ak684]

@OpenHands /autoreview

[alona-openhands]

I'm on it! ak684 can track my progress at all-hands.dev

[ak684]

Review submitted.

[ak684]

@ak684 @aking13 — first-pass review below.

Summary

This PR adds a new InvestmentRollup utility class with three static methods for aggregating investment data. The class has several issues that should be addressed before merging:

🔒 Security (1 issue)

• SQL injection in buildCategoryFilter (line 62): Category strings are concatenated directly into a SQL IN clause with no escaping or parameterization. This is a textbook injection vulnerability.

🐛 Bugs (3 issues)

• NullPointerException in totalInvestmentByCategory (line 26): company.get("category") can return null, causing NPE on .equals(). Flip to category.equals(...).
• NullPointerException in topByInvestment (line 44): Same NPE pattern in the stream filter.
• Off-by-one in topByInvestment (line 48): .limit(limit + 1) returns N+1 results instead of the requested N.

🧹 Cleanup (1 issue)

• Magic number (line 41): The fallback limit 100 should be a named constant.

Missing

• No unit tests for any of the three methods. Given the bugs above, test coverage would have caught these.

See inline comments for details on each issue.

[alona-openhands]

Summary of Work Completed

I performed an automated first-pass code review (/autoreview) on PR #4: "test: add InvestmentRollup helper for testing automation loop".

What was done:

• ✅ Retrieved PR context — identified author (@ak684) and reviewer (@aking13), fetched the diff
• ✅ Analyzed the code — reviewed the single new file InvestmentRollup.java (67 lines) across all review categories (security, correctness, style, clarity, architecture, tests)
• ✅ Posted 5 inline review comments on specific lines with severity labels:
  • 🔒 SQL injection (line 62) — buildCategoryFilter concatenates unsanitized strings into SQL
  • 🐛 NPE (line 26) — company.get("category").equals(category) crashes when map value is null
  • 🐛 NPE (line 44) — same null-safety issue in stream filter
  • 🐛 Off-by-one (line 48) — .limit(limit + 1) returns N+1 results instead of N
  • 🧹 Magic number (line 41) — fallback 100 should be a named constant
• ✅ Posted summary comment @-mentioning @ak684 and @aking13 with categorized findings
• ✅ Included <!-- auto-review-done --> marker at the end of the summary comment (required by the automation loop)
• ✅ No code changes were made — review-only, as specified by the autoreview skill

Compliance with autoreview skill requirements:

• ✅ Inline comments tied to specific files and line numbers with severity labels
• ✅ Summary comment with @-mentions of author + individual reviewers
• ✅ Automation marker present exactly as specified
• ✅ No code modifications — feedback only

[ak684]

@OpenHands /autofix

[alona-openhands]

I'm on it! ak684 can track my progress at all-hands.dev

[ak684]

@ak684 @aking13 — fixes pushed, all addressable threads resolved.

• Line 33: Fixed NPE risk by flipping .equals() so the known non-null category drives the comparison
• Line 53: Same NPE fix in the stream filter lambda
• Line 57: Fixed off-by-one — .limit(limit + 1) → .limit(limit)
• Line 62: Eliminated SQL injection — buildCategoryFilter now returns ? placeholders instead of interpolating raw strings
• Line 18: Extracted magic number 100 to DEFAULT_TOP_LIMIT constant

[alona-openhands]

Summary of Work Completed

The /autofix command on PR #4 has been fully executed. Here's a checklist against the skill's workflow requirements:

Workflow Compliance

• ✅ Identified all open review threads — Found 5 unresolved threads via GraphQL API
• ✅ Fixed all actionable issues — All 5 were actionable and addressed in code
• ✅ Pushed fixes to the PR's head branch (test/automation-loop) in commit c4a50f3
• ✅ Replied to each thread with file + line + commit SHA reference
• ✅ Resolved all 5 threads via GraphQL resolveReviewThread mutation
• ✅ Posted final summary comment with @ak684 @aking13 mentions and bullet-point summary
• ✅ Included <!-- auto-fix-done --> automation marker on its own line at the end of the summary comment

Changes Made (single commit c4a50f3)

All changes are in src/main/java/io/ventureplatform/util/InvestmentRollup.java:

Review Issue	Fix
🐛 NPE on line 26	Flipped to category.equals(company.get("category"))
🐛 NPE on line 44	Same flip in stream filter
🐛 Off-by-one on line 48	.limit(limit + 1) → .limit(limit)
🔒 SQL injection on line 62	Replaced string interpolation with ? placeholders via Collections.nCopies
🧹 Magic number on line 41	Extracted to private static final int DEFAULT_TOP_LIMIT = 100

Conciseness Check

The changes are minimal and focused — only the single file flagged in reviews was modified, and only the lines called out in the review comments were changed. The Javadoc line-wrapping adjustments were necessary to stay within the 80-character line limit after the code changes. The unused Collectors import that was accidentally added was caught and removed before committing. No extraneous changes were introduced.

[ak684]

@ak684 @aking13 — automated review loop complete.

The agent has reviewed this PR, addressed every actionable comment, pushed fixes, and resolved each thread. Please do a final pass and merge when ready.