feat: Pause point responses now show the actual resolved source line - #1849
Conversation
The resolved line can be rounded forward from the requested line (the resolver picks the closest sequence point on or after it), and the caller had no way to notice a mismatch without manually re-reading the source file. - Add SourcePausePointSourceLineReader to read a single line's trimmed text from disk - Surface it as ResolvedLineText in enable-pause-point's response, alongside the existing ResolvedLine
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPause-point enabling by file and line now returns the trimmed text of the resolved source line. A dedicated reader handles invalid paths and line numbers, while tests cover reader behavior and response integration. ChangesPause-point source text
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant EnableBySourceLocation
participant SourcePausePointSourceLineReader
participant SourceFile
Caller->>EnableBySourceLocation: Enable by file and line
EnableBySourceLocation->>SourcePausePointSourceLineReader: Read resolved source line
SourcePausePointSourceLineReader->>SourceFile: Load line text
SourceFile-->>SourcePausePointSourceLineReader: Return source line
SourcePausePointSourceLineReader-->>EnableBySourceLocation: Return trimmed text
EnableBySourceLocation-->>Caller: Return response with ResolvedLineText
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointSourceLineReader.cs (1)
18-24: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valuePrefer
File.ReadLinesoverFile.ReadAllLinesfor memory efficiency.
File.ReadAllLinesreads the entire file into memory at once. While typical C# source files are small enough that this isn't a strict problem, usingFile.ReadLinescombined with LINQ avoids allocating a large array by streaming the file lazily until the target line is found.♻️ Proposed refactor
First, add
using System.Linq;to the using directives. Then apply:- string[] lines = File.ReadAllLines(absoluteFilePath); - if (lineNumber > lines.Length) - { - return string.Empty; - } - - return lines[lineNumber - 1].Trim(); + string line = File.ReadLines(absoluteFilePath).Skip(lineNumber - 1).FirstOrDefault(); + return line != null ? line.Trim() : string.Empty;🤖 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 `@Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointSourceLineReader.cs` around lines 18 - 24, Update the line-reading logic in the relevant pause-point source-line reader to use File.ReadLines with LINQ, selecting only the requested line instead of loading the entire file via File.ReadAllLines. Preserve the existing empty-string result when the requested line is beyond the file and continue trimming the returned line.
🤖 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.
Nitpick comments:
In
`@Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointSourceLineReader.cs`:
- Around line 18-24: Update the line-reading logic in the relevant pause-point
source-line reader to use File.ReadLines with LINQ, selecting only the requested
line instead of loading the entire file via File.ReadAllLines. Preserve the
existing empty-string result when the requested line is beyond the file and
continue trimming the returned line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9dd75e31-2590-4c1f-9175-d5a7333b41c1
⛔ Files ignored due to path filters (2)
Assets/Tests/Editor/SourcePausePointSourceLineReaderTests.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointSourceLineReader.cs.metais excluded by none and included by none
📒 Files selected for processing (4)
Assets/Tests/Editor/PausePointTests.csAssets/Tests/Editor/SourcePausePointSourceLineReaderTests.csPackages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.csPackages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointSourceLineReader.cs
Addresses a CodeRabbit review nitpick on PR #1849: File.ReadLines skips to the target line lazily instead of materializing every line into an array.
2c995c1
into
feature/pause-point-feedback-integration
Summary
enable-pause-point's response now includes the actual source text of the resolved line, so a pause point that got rounded forward to a different line than requested is immediately visible.User Impact
--linehad no sequence point and the resolver rounded forward to the next executable line, the response only showed the line number. Confirming what code that number actually points to required manually opening the file.ResolvedLineText, the trimmed text of the resolved line, next to the existingResolvedLineandResolvedMethodfields — a rounding mismatch is visible without leaving the response.Changes
SourcePausePointSourceLineReader: reads a single line's trimmed text from disk, degrading to an empty string for missing files or out-of-range lines.PausePointTools.EnableBySourceLocation: setsResolvedLineTexton the response using the resolved line number.Verification
dist/darwin-arm64/uloop compile --project-path <repo>→ 0 errors / 0 warningsdist/darwin-arm64/uloop run-tests --project-path <repo> --filter-type regex --filter-value "PausePoint" --timeout-seconds 240→ 178/178 passeddist/darwin-arm64/uloop enable-pause-point --file "Assets/Tests/Editor/PausePointToolsFixture.cs" --line 12against a running Unity Editor → response includes"ResolvedLineText": "return sum;"