Conversation
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
|
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 configurationConfiguration used: Repository: Resgrid/Core/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe changes adjust background-pull retry timing after failure and make occupancy source collection stop when a POI-type query returns null. ChangesSearch index background pull
Occupancy source collection
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to A failed POI lookup now stops inventory without committing incomplete results, while background pulls retry after failure-based backoff. The added test covers null and empty results; no material merge risk was established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
| // Pois carry no DepartmentId column; a department owns its POIs through their POI type. The repository logs and | ||
| // returns null on failure (an empty query is an empty list), and treating that as "no POIs" would stamp an | ||
| // inventory that never saw them as complete. | ||
| var poiTypes = (await _poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId)) |
There was a problem hiding this comment.
External repository failure in _poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId) can leave the IReadOnlyList<PoiType> poiTypes assignment without department context or return null. Catch Exception, map null to InvalidOperationException, log the failure with _logger.LogError and departmentId, and rethrow the original exception.
Kody rule violation: Add try-catch blocks for external calls
IReadOnlyList<PoiType> poiTypes;
try
{
poiTypes = (await _poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId)) ?? throw new InvalidOperationException("The department's points of interest could not be loaded.");
}
catch (Exception ex)
{
_logger.LogError(ex, "Failed to load POI types for department {DepartmentId}", departmentId);
throw;
}Prompt for LLM
File Core/Resgrid.Services/Records/RecordsOccupancyService.cs:
Line 498:
External repository failure in `_poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId)` can leave the `IReadOnlyList<PoiType> poiTypes` assignment without department context or return null. Catch `Exception`, map null to `InvalidOperationException`, log the failure with `_logger.LogError` and `departmentId`, and rethrow the original exception.
Suggested Code:
IReadOnlyList<PoiType> poiTypes;
try
{
poiTypes = (await _poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId)) ?? throw new InvalidOperationException("The department's points of interest could not be loaded.");
}
catch (Exception ex)
{
_logger.LogError(ex, "Failed to load POI types for department {DepartmentId}", departmentId);
throw;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Func<Task> inventory = () => _h.OccupancyService.InventoryCandidatesAsync(Dept, Admin); | ||
| await inventory.Should().ThrowAsync<InvalidOperationException>(); | ||
| _h.Crosswalks.Rows.Should().BeEmpty(); | ||
| var status = await _h.OccupancyService.GetReconciliationStatusAsync(Dept); |
There was a problem hiding this comment.
Unhandled task rejection from _h.OccupancyService.GetReconciliationStatusAsync(Dept) can fail the test without useful context at Tests/Resgrid.Tests/Rms/RecordsOccupancyServiceTests.cs:151, corresponding to Core/Resgrid.Services/Records/RecordsOccupancyService.cs:498. Catch Exception and use Assert.Fail to report the reconciliation-status retrieval error.
Kody rule violation: Handle async operations with proper error handling
ReconciliationStatus status;
try
{
status = await _h.OccupancyService.GetReconciliationStatusAsync(Dept);
}
catch (Exception ex)
{
Assert.Fail($"Failed to retrieve reconciliation status: {ex.Message}");
return;
}Prompt for LLM
File Tests/Resgrid.Tests/Rms/RecordsOccupancyServiceTests.cs:
Line 146:
Unhandled task rejection from `_h.OccupancyService.GetReconciliationStatusAsync(Dept)` can fail the test without useful context at `Tests/Resgrid.Tests/Rms/RecordsOccupancyServiceTests.cs:151`, corresponding to `Core/Resgrid.Services/Records/RecordsOccupancyService.cs:498`. Catch `Exception` and use `Assert.Fail` to report the reconciliation-status retrieval error.
Suggested Code:
ReconciliationStatus status;
try
{
status = await _h.OccupancyService.GetReconciliationStatusAsync(Dept);
}
catch (Exception ex)
{
Assert.Fail($"Failed to retrieve reconciliation status: {ex.Message}");
return;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2 participants
Summary
Functional Impact