fix: revert to stable baseline with minimal targeted improvements
This commit is contained in:
+8
-57
@@ -149,13 +149,8 @@ async function runOllamaLoop(ctx: AgentRunContext): Promise<AgentResult> {
|
|||||||
let calledOutputTool = false;
|
let calledOutputTool = false;
|
||||||
let addedContinueNudge = false;
|
let addedContinueNudge = false;
|
||||||
let reportProgressCount = 0;
|
let reportProgressCount = 0;
|
||||||
let selectedModeName = "";
|
|
||||||
// Accumulates content from each read_file call (all ranges) so that when
|
|
||||||
// tool messages are pruned, the model retains the diff content it read.
|
|
||||||
// Keyed by "path:start:end" so multiple ranges of the same file are kept.
|
|
||||||
const readFileLog: string[] = [];
|
|
||||||
let injectedFileReadSummary = false;
|
|
||||||
let editCommentCount = 0;
|
let editCommentCount = 0;
|
||||||
|
let selectedModeName = "";
|
||||||
|
|
||||||
while (iterations < MAX_ITERATIONS) {
|
while (iterations < MAX_ITERATIONS) {
|
||||||
iterations++;
|
iterations++;
|
||||||
@@ -211,23 +206,6 @@ async function runOllamaLoop(ctx: AgentRunContext): Promise<AgentResult> {
|
|||||||
);
|
);
|
||||||
if (promptTokens > numCtx * 0.77) {
|
if (promptTokens > numCtx * 0.77) {
|
||||||
messages = pruneToolMessages(messages);
|
messages = pruneToolMessages(messages);
|
||||||
// Inject accumulated diff content as a user message (survives pruning).
|
|
||||||
// Only inject once — subsequent prune events leave this message intact.
|
|
||||||
if (readFileLog.length > 0 && !injectedFileReadSummary) {
|
|
||||||
injectedFileReadSummary = true;
|
|
||||||
const combined = readFileLog.join("\n---\n").slice(0, 10000);
|
|
||||||
const isReview =
|
|
||||||
selectedModeName === "Review" || selectedModeName === "IncrementalReview";
|
|
||||||
messages.push({
|
|
||||||
role: "user",
|
|
||||||
content:
|
|
||||||
`Diff content read earlier (preserved after context pruning):\n\n${combined}\n\n` +
|
|
||||||
(isReview
|
|
||||||
? "Now call create_pull_request_review with your review findings. Do NOT call report_progress."
|
|
||||||
: "Continue the workflow using the content above."),
|
|
||||||
});
|
|
||||||
log.info(`» injected file-read summary: ${readFileLog.length} read(s)`);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -308,20 +286,6 @@ async function runOllamaLoop(ctx: AgentRunContext): Promise<AgentResult> {
|
|||||||
content: result,
|
content: result,
|
||||||
});
|
});
|
||||||
|
|
||||||
// Track every read_file call — accumulate all ranges (same path, different
|
|
||||||
// start/end lines) since the model reads the diff file in multiple chunks.
|
|
||||||
// read_file returns { content: "..." } JSON — extract the actual content.
|
|
||||||
if (toolName === "read_file") {
|
|
||||||
let excerpt: string;
|
|
||||||
try {
|
|
||||||
const parsed = JSON.parse(result) as { content?: string };
|
|
||||||
excerpt = (parsed.content ?? result).slice(0, 500);
|
|
||||||
} catch {
|
|
||||||
excerpt = result.slice(0, 500);
|
|
||||||
}
|
|
||||||
readFileLog.push(excerpt);
|
|
||||||
}
|
|
||||||
|
|
||||||
// After checkout_pr returns, extract and pin the diffPath as a user
|
// After checkout_pr returns, extract and pin the diffPath as a user
|
||||||
// message. Only tool messages are pruned — user messages survive context
|
// message. Only tool messages are pruned — user messages survive context
|
||||||
// pressure — so the model retains the exact diff file path and knows how
|
// pressure — so the model retains the exact diff file path and knows how
|
||||||
@@ -347,30 +311,17 @@ async function runOllamaLoop(ctx: AgentRunContext): Promise<AgentResult> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// report_progress is forbidden in Review/IncrementalReview — intercept it.
|
// report_progress called repeatedly means the model is stuck.
|
||||||
// In other modes, stop after 2+ calls (loop detection).
|
|
||||||
if (toolName === "report_progress") {
|
if (toolName === "report_progress") {
|
||||||
const isReview =
|
reportProgressCount++;
|
||||||
selectedModeName === "Review" || selectedModeName === "IncrementalReview";
|
if (reportProgressCount >= 2) {
|
||||||
if (isReview) {
|
log.info("» report_progress loop — stopping");
|
||||||
log.info("» report_progress intercepted in Review mode — redirecting");
|
await unloadModel(ollama, model);
|
||||||
messages.push({
|
return { success: true };
|
||||||
role: "user",
|
|
||||||
content:
|
|
||||||
"report_progress is not allowed in Review mode. " +
|
|
||||||
"Call create_pull_request_review now with your findings.",
|
|
||||||
});
|
|
||||||
} else {
|
|
||||||
reportProgressCount++;
|
|
||||||
if (reportProgressCount >= 2) {
|
|
||||||
log.info("» report_progress loop — stopping");
|
|
||||||
await unloadModel(ollama, model);
|
|
||||||
return { success: true };
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// edit_issue_comment called repeatedly means the model is stuck looping.
|
// edit_issue_comment called repeatedly means the model is stuck.
|
||||||
if (toolName === "edit_issue_comment") {
|
if (toolName === "edit_issue_comment") {
|
||||||
editCommentCount++;
|
editCommentCount++;
|
||||||
if (editCommentCount >= 2) {
|
if (editCommentCount >= 2) {
|
||||||
|
|||||||
@@ -64,21 +64,6 @@ state before acting on findings.
|
|||||||
|
|
||||||
Pull every metadata field from the \`checkout_pr\` tool's response — file count, commit count, base/head ref + SHA, the commit list. For \`IncrementalReview\` runs, populate \`Prior shockbot review\` with the prior review's commit_id (short SHA) and \`html_url\` from \`list_pull_request_reviews\`.
|
Pull every metadata field from the \`checkout_pr\` tool's response — file count, commit count, base/head ref + SHA, the commit list. For \`IncrementalReview\` runs, populate \`Prior shockbot review\` with the prior review's commit_id (short SHA) and \`html_url\` from \`list_pull_request_reviews\`.
|
||||||
|
|
||||||
After the metadata comment, include a collapsible per-file summary table built from your file inventory. Place it in the \`body\` parameter of the structured submission, after the metadata comment and before any \`### \` issue sections:
|
|
||||||
|
|
||||||
\`\`\`
|
|
||||||
<details>
|
|
||||||
<summary>Show a summary per file</summary>
|
|
||||||
|
|
||||||
| File | Description |
|
|
||||||
| ---- | ----------- |
|
|
||||||
| \`path/to/file.ts\` | One-line description of what this file's change does |
|
|
||||||
|
|
||||||
</details>
|
|
||||||
\`\`\`
|
|
||||||
|
|
||||||
Cover every non-trivial file you inventoried. If you skipped any (lockfiles, generated files), note "(N files not reviewed)" in the \`<summary>\` tag. Do NOT put this table in the \`changes\` array — \`changes\` is for the high-level bullet list of substantive changes, not the per-file breakdown.
|
|
||||||
|
|
||||||
## 2. Cross-cutting issue sections (zero or more)
|
## 2. Cross-cutting issue sections (zero or more)
|
||||||
|
|
||||||
For each cross-cutting concern, one \`### \` section. Use this exact shape:
|
For each cross-cutting concern, one \`### \` section. Use this exact shape:
|
||||||
@@ -315,8 +300,6 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`,
|
|||||||
|
|
||||||
2. **checkout**: call \`${t("checkout_pr")}\` — this returns PR metadata and a \`diffPath\`. read the diff TOC end-to-end and treat its file line ranges as your coverage checklist.
|
2. **checkout**: call \`${t("checkout_pr")}\` — this returns PR metadata and a \`diffPath\`. read the diff TOC end-to-end and treat its file line ranges as your coverage checklist.
|
||||||
|
|
||||||
**file inventory**: for every non-trivial file in the diff TOC (skip lockfiles, \`*.lock\`, auto-generated migration checksums, formatter/linter configs with no logic changes), read its diff hunk by calling \`read_file\` on \`diffPath\` with \`start_line\`/\`end_line\` from the TOC (e.g. if the TOC shows \`foo.ts (lines 42-100)\`, call \`read_file(path=diffPath, start_line=42, end_line=100)\`). **Do NOT read full source files one-by-one — that exhausts the context window on large PRs.** Only call \`read_file\` on the actual source file when the diff hunk is insufficient to understand broader context. For each file record a one-line description: *what changed and why*. This inventory becomes the per-file table in the review body.
|
|
||||||
|
|
||||||
3. **triage**: orient yourself on the PR — identify *what kind of thing this is* (domain it touches, seams it crosses, external contracts it depends on, user-facing surfaces it changes). pull as much context as you need to render a confident, well-grounded review: read related files, grep for callers of changed symbols, check tests that exercise the touched paths, fetch related GitHub state. **you are the synthesizer** — never delegate understanding to subagents.
|
3. **triage**: orient yourself on the PR — identify *what kind of thing this is* (domain it touches, seams it crosses, external contracts it depends on, user-facing surfaces it changes). pull as much context as you need to render a confident, well-grounded review: read related files, grep for callers of changed symbols, check tests that exercise the touched paths, fetch related GitHub state. **you are the synthesizer** — never delegate understanding to subagents.
|
||||||
|
|
||||||
if the PR is **genuinely trivial**, skip the fan-out entirely and submit a \`No new issues found.\` review per step 7.
|
if the PR is **genuinely trivial**, skip the fan-out entirely and submit a \`No new issues found.\` review per step 7.
|
||||||
@@ -473,8 +456,6 @@ ${PR_SUMMARY_FORMAT}`,
|
|||||||
|
|
||||||
2. **checkout**: call \`${t("checkout_pr")}\` — this returns PR metadata, \`diffPath\` (full diff), and \`incrementalDiffPath\` (changes since last reviewed version, if available). read the diff TOC first and use its line ranges as your coverage checklist.
|
2. **checkout**: call \`${t("checkout_pr")}\` — this returns PR metadata, \`diffPath\` (full diff), and \`incrementalDiffPath\` (changes since last reviewed version, if available). read the diff TOC first and use its line ranges as your coverage checklist.
|
||||||
|
|
||||||
**file inventory**: for every non-trivial file in the incremental diff TOC (skip lockfiles, \`*.lock\`, auto-generated migration checksums, formatter/linter configs with no logic changes), read its diff hunk by calling \`read_file\` on \`diffPath\` (or \`incrementalDiffPath\` if present) with \`start_line\`/\`end_line\` from the TOC. **Do NOT read full source files one-by-one — that exhausts the context window.** Only read the actual source file when the diff hunk is insufficient to understand broader context. Record a one-line description per file: *what changed and why*. This inventory becomes the per-file table in the review body.
|
|
||||||
|
|
||||||
3. **incremental scope**: if \`incrementalDiffPath\` is present, read it to see what changed since the last review. this is a range-diff that isolates the net changes, filtering out base branch noise. if not present, fall back to reviewing the full PR diff and determine what changed since Pullfrog's most recent review.
|
3. **incremental scope**: if \`incrementalDiffPath\` is present, read it to see what changed since the last review. this is a range-diff that isolates the net changes, filtering out base branch noise. if not present, fall back to reviewing the full PR diff and determine what changed since Pullfrog's most recent review.
|
||||||
|
|
||||||
4. **prior feedback — read AND retire it**: fetch previous reviews via \`${t("list_pull_request_reviews")}\`, then call \`${t("get_review_comments")}\` on each prior shockbot review. Each thread renders as a section whose first line is a fenced tag \`comment author=<login> id=<fullDatabaseId> review=<reviewId> thread=<graphqlId>\`; section headers carry \`[RESOLVED]\` / \`[OUTDATED]\` when relevant. For every **open, Pullfrog-originated** thread, decide and act:
|
4. **prior feedback — read AND retire it**: fetch previous reviews via \`${t("list_pull_request_reviews")}\`, then call \`${t("get_review_comments")}\` on each prior shockbot review. Each thread renders as a section whose first line is a fenced tag \`comment author=<login> id=<fullDatabaseId> review=<reviewId> thread=<graphqlId>\`; section headers carry \`[RESOLVED]\` / \`[OUTDATED]\` when relevant. For every **open, Pullfrog-originated** thread, decide and act:
|
||||||
|
|||||||
Reference in New Issue
Block a user