Improve context limits and PR comment fallbacks

Author: haasonsaas

.github/workflows/diffscope.yml

@@ -41,19 +41,38 @@ jobs:
             const fs = require('fs');
             const comments = JSON.parse(fs.readFileSync('comments.json', 'utf8'));
             const headSha = context.payload.pull_request.head.sha;
+            const fallback = [];
             
             for (const comment of comments) {
               if (!comment.file_path || !comment.line_number || comment.line_number < 1) {
                 continue;
               }
-              await github.rest.pulls.createReviewComment({
+              try {
+                await github.rest.pulls.createReviewComment({
+                  owner: context.repo.owner,
+                  repo: context.repo.repo,
+                  pull_number: context.issue.number,
+                  body: `**${comment.severity}**: ${comment.content}`,
+                  commit_id: headSha,
+                  path: comment.file_path,
+                  line: comment.line_number,
+                  side: "RIGHT"
+                });
+              } catch (error) {
+                fallback.push(`- **${comment.severity}** ${comment.file_path}:${comment.line_number} ${comment.content}`);
+              }
+            }
+
+            if (fallback.length > 0) {
+              const body = [
+                "Some review comments could not be placed inline and are listed below:",
+                "",
+                ...fallback.slice(0, 100)
+              ].join("\\n");
+              await github.rest.issues.createComment({
                 owner: context.repo.owner,
                 repo: context.repo.repo,
-                pull_number: context.issue.number,
-                body: `**${comment.severity}**: ${comment.content}`,
-                commit_id: headSha,
-                path: comment.file_path,
-                line: comment.line_number,
-                side: "RIGHT"
+                issue_number: context.issue.number,
+                body
               });
             }

src/core/context.rs

@@ -53,7 +53,10 @@ impl ContextFetcher {
                 let end_idx = end.min(file_lines.len());
 
                 if start_idx < file_lines.len() {
-                    let chunk_content = file_lines[start_idx..end_idx].join("\n");
+                    let chunk_content = truncate_with_notice(
+                        file_lines[start_idx..end_idx].join("\n"),
+                        MAX_CONTEXT_CHARS,
+                    );
                     chunks.push(LLMContextChunk {
                         file_path: file_path.clone(),
                         content: chunk_content,
@@ -106,6 +109,7 @@ impl ContextFetcher {
                 .take(max_lines)
                 .collect::<Vec<_>>()
                 .join("\n");
+            let snippet = truncate_with_notice(snippet, MAX_CONTEXT_CHARS);
             if snippet.trim().is_empty() {
                 continue;
             }
@@ -153,7 +157,10 @@ impl ContextFetcher {
                             // Extract a few lines around the definition for context
                             let start_line = line_num.saturating_sub(2);
                             let end_line = (line_num + 5).min(lines.len());
-                            let definition_content = lines[start_line..end_line].join("\n");
+                            let definition_content = truncate_with_notice(
+                                lines[start_line..end_line].join("\n"),
+                                MAX_CONTEXT_CHARS,
+                            );
 
                             chunks.push(LLMContextChunk {
                                 file_path: file_path.clone(),
@@ -194,6 +201,17 @@ fn merge_ranges(lines: &[(usize, usize)]) -> Vec<(usize, usize)> {
     merged
 }
 
+const MAX_CONTEXT_CHARS: usize = 8000;
+
+fn truncate_with_notice(mut content: String, max_chars: usize) -> String {
+    if max_chars == 0 || content.len() <= max_chars {
+        return content;
+    }
+    content.truncate(max_chars.saturating_sub(20));
+    content.push_str("\n[Truncated]\n");
+    content
+}
+
 async fn read_file_lossy(path: &Path) -> Result<String> {
     match tokio::fs::read_to_string(path).await {
         Ok(content) => Ok(content),

src/core/prompt.rs

@@ -8,6 +8,7 @@ pub struct PromptConfig {
     pub user_prompt_template: String,
     pub max_tokens: usize,
     pub include_context: bool,
+    pub max_context_chars: usize,
 }
 
 impl Default for PromptConfig {
@@ -50,6 +51,7 @@ Line 28: Performance - O(n²) algorithm for large dataset. Will be slow with man
 </instructions>"#.to_string(),
             max_tokens: 2000,
             include_context: true,
+            max_context_chars: 20000,
         }
     }
 }
@@ -108,7 +110,7 @@ impl PromptBuilder {
         let mut output = String::new();
 
         for chunk in chunks {
-            output.push_str(&format!(
+            let block = format!(
                 "\n[{:?} - {}{}]\n{}\n",
                 chunk.context_type,
                 chunk.file_path.display(),
@@ -117,7 +119,14 @@ impl PromptBuilder {
                     .map(|(s, e)| format!(":{}-{}", s, e))
                     .unwrap_or_default(),
                 chunk.content
-            ));
+            );
+            if self.config.max_context_chars > 0
+                && output.len().saturating_add(block.len()) > self.config.max_context_chars
+            {
+                output.push_str("\n[Context truncated]\n");
+                break;
+            }
+            output.push_str(&block);
         }
 
         Ok(output)

src/core/smart_review_prompt.rs

@@ -74,6 +74,8 @@ TAGS: [comma-separated relevant tags]
         context_chunks: &[LLMContextChunk],
     ) -> Result<String> {
         let mut prompt = String::new();
+        let mut context_chars = 0usize;
+        const MAX_CONTEXT_CHARS: usize = 20000;
 
         prompt.push_str(&format!(
             "Please review the following code changes in file: {}\n\n",
@@ -90,7 +92,7 @@ TAGS: [comma-separated relevant tags]
                     chunk.file_path.display(),
                     format!("{:?}", chunk.context_type)
                 );
-                prompt.push_str(&format!(
+                let block = format!(
                     "**{}** (lines {}-{}):\n```\n{}\n```\n\n",
                     description,
                     start_line,
@@ -101,7 +103,13 @@ TAGS: [comma-separated relevant tags]
                         .take(20)
                         .collect::<Vec<_>>()
                         .join("\n")
-                ));
+                );
+                if context_chars.saturating_add(block.len()) > MAX_CONTEXT_CHARS {
+                    prompt.push_str("[Context truncated]\n\n");
+                    break;
+                }
+                prompt.push_str(&block);
+                context_chars = context_chars.saturating_add(block.len());
             }
         }
 

src/main.rs

@@ -375,6 +375,7 @@ async fn review_command(
                 }
             }
 
+            let comments = filter_comments_for_diff(&diff, comments);
             all_comments.extend(comments);
         }
     }
@@ -888,6 +889,7 @@ async fn review_diff_content_raw(
                 }
             }
 
+            let comments = filter_comments_for_diff(&diff, comments);
             all_comments.extend(comments);
         }
     }
@@ -1308,6 +1310,7 @@ async fn smart_review_command(
                 }
             }
 
+            let comments = filter_comments_for_diff(&diff, comments);
             all_comments.extend(comments);
         }
     }
@@ -1777,6 +1780,41 @@ fn extract_symbols_from_diff(diff: &core::UnifiedDiff) -> Vec<String> {
     symbols
 }
 
+fn filter_comments_for_diff(
+    diff: &core::UnifiedDiff,
+    comments: Vec<core::Comment>,
+) -> Vec<core::Comment> {
+    let mut filtered = Vec::new();
+    let total = comments.len();
+    for comment in comments {
+        if is_line_in_diff(diff, comment.line_number) {
+            filtered.push(comment);
+        }
+    }
+
+    if filtered.len() != total {
+        let dropped = total.saturating_sub(filtered.len());
+        info!(
+            "Dropped {} comment(s) for {} due to unmatched line numbers",
+            dropped,
+            diff.file_path.display()
+        );
+    }
+
+    filtered
+}
+
+fn is_line_in_diff(diff: &core::UnifiedDiff, line_number: usize) -> bool {
+    if line_number == 0 {
+        return false;
+    }
+    diff.hunks.iter().any(|hunk| {
+        hunk.changes
+            .iter()
+            .any(|line| line.new_line_no == Some(line_number))
+    })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;