learnings: TOC + section taxonomy + 100k cap, hygiene rules, tool-quirk descriptions (#717)
* audit learnings: reshape reflection prompt + bake tool quirks into descriptions (#619) Cross-repo audit of the 48 repos with non-null learnings turned up two recurring failure modes: 1. ~25-30% of bullets across the most-active repos are pullfrog-tool quirks ("shell timeout is in milliseconds", "git args must be a JSON array", "create_pull_request_review drops out-of-hunk comments", "push_branch may report timeout when push succeeded", "checkout_pr shallow.lock retries", "commit_id needs full 40-char SHA"). These are universal across repos and should live in tool descriptions, not be rediscovered and stored 48 times. Tool descriptions now surface them. 2. Bullets are routinely 200-1000 chars (paragraph-length), and 12 of 48 repos are at the 10k cap. The reflection prompt now: caps bullets at ~240 chars (one specific fact), bans PR/review/commit/date-anchored facts that decay within weeks, bans tool-quirk learnings, and tells the agent that cap pressure means compress+prune existing bullets, not skip new findings. Co-authored-by: Cursor <cursoragent@cursor.com> * learnings: add server-generated TOC, fixed section taxonomy, raise cap to 100k (#707) Cap goes 10k → 100k. Reads stay bounded because the seeded file now opens with a server-generated table of contents listing every `## ` section's line range — agents read the TOC, then `read_file offset/limit` just the sections relevant to the current task instead of slurping the whole file. ## Section taxonomy (fixed) `## Build & test`, `## CI`, `## Conventions`, `## Architecture`, `## Gotchas`. Free-form `### ` sub-headings inside a section are fine. Pre-taxonomy free-text rows get wrapped in a `## Legacy` carve-out on first seed so they remain visible while the agent gradually re-curates them during reflection turns. ## Storage shape unchanged `Repo.learnings` still holds raw markdown (no schema migration). The TOC is a pure read-side affordance: prepended at seed time, stripped from the agent-edited file before persist. Markers `<!-- pullfrog-learnings-toc:* -->` delimit the strip region. Agent edits inside the markers are discarded. ## Round-trip semantics `seedLearningsFile` now returns `{ path, canonicalSeed }` where `canonicalSeed` is the post-TOC body — same shape `readLearningsFile` returns at end-of-run, so `persistLearnings` byte-compares them directly to skip the no-op PATCH. Empty-repo first runs end up with the section scaffold both as seed and as read-back, so untouched runs still short-circuit cleanly. ## Reflection prompt Adds explicit section-placement guidance (place each new bullet under the most relevant `## `; do NOT add new top-level headings; do NOT edit anything between the TOC markers). Carries forward the bullet hygiene from the previous commit: ≤240 chars per bullet, no pullfrog-tool quirks (those belong in tool descriptions), no PR/review/commit/date references. The "near cap" framing is replaced with "compress and prune within a section when it grows noisy" since the cap pressure that drove cramming is gone. Co-authored-by: Cursor <cursoragent@cursor.com> * anneal round 1: line-anchored taxonomy detect, partial-merge, line-boundary truncation, scaffold-empty UI Multi-lens review of the TOC + taxonomy diff surfaced a cluster of correctness and operational bugs. Fixes: - `hasAnyTaxonomyHeading` used `String.includes("## X")` which false-positives on `### X` (the `## ` substring sits inside `### `), prose containing `## CI`, fenced code documenting markdown, etc. Replaced with a line-anchored predicate that reuses `parseHeadings` so detection and TOC construction stay consistent. - The "any heading present → pass through verbatim" rule meant a body with one taxonomy heading would seed without the other four. Worse, requiring all five would flip a body back into Legacy when the agent legitimately pruned a section to empty. New `partial` kind: keep existing content in place, append missing sections in canonical order so the agent always has the full scaffold without losing pruning intent. - `stripLearningsToc` collapsed `\n{3,}` globally; `canonicalSeed` doesn't, so an untouched body with intentional triple-newline spacing would compare unequal and burn a spurious LearningsRevision row each run. Drop the global collapse — only the leading newlines that the strip itself introduces are normalized. - 100k truncation via `slice(0, 100_000)` could cut mid-line, breaking `parseHeadings` (whole-line `^## `) on the next seed and flipping a cut body back into Legacy. New `truncateAtLineBoundary` cuts at the last newline before the cap. - `LearningsSection.tsx` rendered a scaffold-only body as "has learnings" instead of the empty placeholder. Added a `hasOnlyEmptyScaffold` guard so the console behaves the same as pre-PR for the empty case. - Seed log line distinguishes `kind=structured/partial/legacy-wrapped/ empty` instead of `existing=yes/no`, so operators can spot legacy migration activity in logs. - New tests cover: substring false-positive (`### Build & test`, in-prose mentions), partial-taxonomy merge (no Legacy wrap), full-taxonomy structured pass-through, last-newline truncation, triple-newline preservation. Deferred (documented in PR body): deploy-ordering footgun (action before API), rollback for rows >10k, Gemini sanitizer dropping `description` on `anyOf` branches, reflection-on-failed-runs. Co-authored-by: Cursor <cursoragent@cursor.com> * anneal r2: hard-truncate fallback when line boundary discards >4k Round-2 review caught a regression in `truncateAtLineBoundary`: when the only newline within the first 100k chars sits near the start (e.g. one heading + 100k+ char single line — pathological pasted log dumps), the line-boundary cut discards almost all of the body. losing one partial line is preferable to losing kilobytes; threshold the fallback at 4k. Co-authored-by: Cursor <cursoragent@cursor.com> * move TOC out of file: prompt-side rendering, server-parsed headings drops the in-file TOC + fixed taxonomy in favor of: - file on disk = verbatim Repo.learnings (no markers, no scaffold) - server parses headings (mdast-util-from-markdown) at run-context time and returns them as RepoSettings.learningsHeadings - action renders heading TOC into the LEARNINGS prompt section as parenthesized line ranges like `Build & test (L1-L42)` with hierarchy via 2-space indent off the shallowest depth - reflection prompt teaches agent-curated structure with a soft 300-line per-section cap and explicit guidance to restructure flat legacy lists cuts 8 helpers (ensureSections, stripLearningsToc, assembleFile, buildTocBlock, parseHeadings, buildSectionScaffold, hasAnyTaxonomyHeading, LEARNINGS_SECTIONS) and the canonicalSeed round-trip dance. action seedLearningsFile is now { path } only; main.ts byte-compares the trimmed read-back against (current ?? "").trim() to gate the persist PATCH. truncateAtLineBoundary kept for safety. new tests: - test/learningsToc.test.ts (11 parser cases incl. fenced-code, blockquote, arbitrary h1-h6 nesting, startLine-points-at-heading invariant) - action/utils/learningsTocRender.test.ts (7 renderer cases) --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Colin McDonnell <colinmcd94@gmail.com>
This commit is contained in:
David Blass
committed by
pullfrog[bot]
pullfrog[bot]
parent
d04c1ca3da
commit
5518890b18
+29
-5
@@ -237,6 +237,16 @@ export function buildPostRunPrompt(issues: PostRunIssues): string {
|
||||
* the file is the single source of truth — there is no separate MCP tool
|
||||
* call. the server reads the file at end-of-run and persists any edits to
|
||||
* `Repo.learnings`.
|
||||
*
|
||||
* the prompt copy is shaped by repo-wide audits of the actual content the
|
||||
* agent has been writing (issue #619 in pullfrog/app). recurring failure
|
||||
* modes the framing pushes back on:
|
||||
* - massive multi-paragraph "bullets" that are really mini-articles
|
||||
* - PR-/review-/commit-/date-anchored facts that decay within weeks
|
||||
* - rediscovery of pullfrog-tool quirks that belong in tool descriptions,
|
||||
* not per-repo learnings
|
||||
* - sections growing into giant flat lists with no internal structure,
|
||||
* forcing future runs to read kilobytes to find one fact
|
||||
*/
|
||||
export function buildLearningsReflectionPrompt(filePath: string): string {
|
||||
return [
|
||||
@@ -244,11 +254,25 @@ export function buildLearningsReflectionPrompt(filePath: string): string {
|
||||
"",
|
||||
`the rolling learnings file is at \`${filePath}\`. read it first if you haven't already, then edit it in place using your native file tools. the server reads this file at end-of-run and persists any changes — there is no tool to call.`,
|
||||
"",
|
||||
`keep the file healthy:`,
|
||||
`- only add bullets when the finding is high-confidence AND broadly useful. skip speculative, one-off, or "maybe" findings.`,
|
||||
`- prune bullets that are clearly wrong, no longer relevant, or low-signal (rarely useful). a focused, accurate file beats a long stale one.`,
|
||||
`- format: flat bullet list, one fact per line starting with \`- \`. deduplicate against existing entries — if a bullet covers the same fact, update it in place instead of adding a duplicate.`,
|
||||
`- leave the file alone if you have nothing substantively new to add and the existing entries still look healthy. silence is a valid outcome — just reply "done" and stop.`,
|
||||
`structure:`,
|
||||
`- markdown hierarchy: \`## \` for top-level themes, \`### \` and deeper for sub-themes when a section grows. there is no fixed taxonomy — choose headings that fit THIS repo (e.g. for one repo \`## Migrations\` / \`## Local dev\` may make sense; for another, \`## API quirks\` / \`## Failure modes\`).`,
|
||||
`- **no section over ~300 lines.** when a section is approaching that, split it: introduce \`### \` subsections grouping related bullets, or hoist a coherent group into a new top-level \`## \` section. granular sections mean future runs read targeted line ranges instead of slurping the whole file. this is the most important hygiene rule on long-lived repos.`,
|
||||
`- if you find a flat unstructured list (legacy content from before this format), restructure it: read it, group related bullets, rewrite the file with \`## \` / \`### \` headings around them. don't preserve bad structure — fix it.`,
|
||||
"",
|
||||
`bullet hygiene:`,
|
||||
`- one fact per line starting with \`- \`. each bullet is ONE specific durable fact, not a paragraph or essay.`,
|
||||
`- aim for ≤ 240 chars per bullet. longer bullets are almost always mixing multiple facts that should be split, or burying the durable claim under PR-specific context that should be cut.`,
|
||||
`- only add bullets when the finding is high-confidence AND broadly useful AND will still be true in 3+ months. skip speculative, one-off, or "maybe" findings.`,
|
||||
`- prune bullets that are clearly wrong, no longer relevant, or low-signal. a focused, accurate file beats a long stale one. compressing two overlapping bullets into one tighter bullet counts as progress.`,
|
||||
`- deduplicate against existing entries (in any section) — if a bullet covers the same fact, update it in place instead of adding a duplicate.`,
|
||||
"",
|
||||
`do NOT add bullets for:`,
|
||||
`- pullfrog tool quirks (e.g. "\`shell\` timeout is in milliseconds", "\`git\` args must be a JSON array", "\`create_pull_request_review\` drops out-of-hunk comments", "\`push_branch\` may report timeout when push succeeded"). these are universal across repos and belong in the tool descriptions — flag the gap rather than hoarding the workaround per-repo.`,
|
||||
`- references to specific PR numbers, review IDs, commit SHAs, branch names, or person handles ("PR #595 introduced X", "flagged in review 12345", "as of commit abc123"). repo state changes; these decay into noise within weeks.`,
|
||||
`- dated assertions ("as of May 2026", "currently...", "for now..."). if a fact needs a date to be true, it isn't durable enough to belong here.`,
|
||||
`- play-by-play of what THIS run did. learnings are for the NEXT run, not a retrospective.`,
|
||||
"",
|
||||
`if you have nothing substantively new to add AND the existing entries still look healthy and well-structured, leave the file alone — just reply "done" and stop. silence is a valid outcome.`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
|
||||
@@ -770,12 +770,10 @@ export async function main(): Promise<MainResult> {
|
||||
current: runContext.repoSettings.learnings,
|
||||
});
|
||||
toolState.learningsFilePath = learningsPath;
|
||||
try {
|
||||
toolState.learningsSeed = await readFile(learningsPath, "utf8");
|
||||
} catch {
|
||||
// intentionally empty — learningsSeed stays undefined, persistLearnings
|
||||
// will treat seed as "" and persist any non-empty content
|
||||
}
|
||||
// file on disk is the verbatim DB body, so the seed used for
|
||||
// change-detection is just `current ?? ""` (trimmed). persistLearnings
|
||||
// byte-compares against the trimmed read-back to skip no-op PATCHes.
|
||||
toolState.learningsSeed = (runContext.repoSettings.learnings ?? "").trim();
|
||||
log.info(
|
||||
`» learnings seeded at ${learningsPath} (existing=${runContext.repoSettings.learnings ? "yes" : "no"})`
|
||||
);
|
||||
@@ -839,6 +837,7 @@ export async function main(): Promise<MainResult> {
|
||||
agentId,
|
||||
outputSchema,
|
||||
learningsFilePath: toolState.learningsFilePath ?? null,
|
||||
learningsHeadings: runContext.repoSettings.learningsHeadings,
|
||||
});
|
||||
const logParts = [
|
||||
instructions.eventInstructions
|
||||
|
||||
+3
-1
@@ -593,7 +593,9 @@ export function CheckoutPrTool(ctx: ToolContext) {
|
||||
name: "checkout_pr",
|
||||
description:
|
||||
"Checkout a pull request branch locally. This fetches the PR branch and sets up push configuration for fork PRs. " +
|
||||
"Returns diffPath pointing to the formatted diff file.",
|
||||
"Returns diffPath pointing to the formatted diff file. " +
|
||||
"Transient fetch timeouts are common — retry the same call up to a few times before treating the failure as terminal. " +
|
||||
"If the error mentions `.git/shallow.lock: File exists` or `.git/index.lock: File exists`, that's a stale lock from a prior timed-out fetch — remove it via the shell tool (`rm -f .git/shallow.lock .git/index.lock`) and retry.",
|
||||
parameters: CheckoutPr,
|
||||
execute: execute(async ({ pull_number }) => {
|
||||
const prResponse = await ctx.octokit.rest.pulls.get({
|
||||
|
||||
+8
-2
@@ -221,7 +221,8 @@ export function PushBranchTool(ctx: ToolContext) {
|
||||
"If specifying branchName, use the LOCAL branch name (e.g., 'pr-1'), not the remote branch name. " +
|
||||
"The correct remote and remote branch are determined automatically from branch config set by checkout_pr. " +
|
||||
"Requires a clean working tree. Runs the repository prepush hook (if configured) before the network push — hook failure means tests/lint or similar in that script failed, not necessarily a Pullfrog timeout. " +
|
||||
"Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode.",
|
||||
"Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode. " +
|
||||
"If the response reports a timeout, the underlying push may have actually succeeded — verify with `git log origin/<branch>` (or this tool with command 'log') before retrying, otherwise you'll push a duplicate.",
|
||||
parameters: PushBranch,
|
||||
execute: execute(async ({ branchName, force }) => {
|
||||
// permission check
|
||||
@@ -444,7 +445,12 @@ const subcommandPattern = regex("^[a-z][a-z0-9-]*$");
|
||||
|
||||
const Git = type({
|
||||
command: type(subcommandPattern).describe("Git command (e.g., 'status', 'log', 'diff')"),
|
||||
args: type.string.array().describe("Additional arguments for the git command").optional(),
|
||||
args: type.string
|
||||
.array()
|
||||
.describe(
|
||||
'Additional arguments for the git command, as a JSON array of strings (NOT a single string). e.g. args: ["HEAD"], args: ["--oneline", "-20"]. Passing a single string fails validation.'
|
||||
)
|
||||
.optional(),
|
||||
});
|
||||
|
||||
export function GitTool(ctx: ToolContext) {
|
||||
|
||||
+5
-3
@@ -320,14 +320,16 @@ export const CreatePullRequestReview = type({
|
||||
)
|
||||
.optional(),
|
||||
commit_id: type.string
|
||||
.describe("Optional SHA of the commit being reviewed. Defaults to latest.")
|
||||
.describe(
|
||||
"Optional SHA of the commit being reviewed. Defaults to latest. Must be the FULL 40-character SHA — abbreviated SHAs are rejected by GitHub with `422 Unprocessable Entity`. The PR-synchronize event payload's `head_sha` is already full-length."
|
||||
)
|
||||
.optional(),
|
||||
comments: type({
|
||||
path: type.string.describe(
|
||||
"The file path to comment on (relative to repo root). Must be a file that appears in the PR diff."
|
||||
),
|
||||
line: type.number.describe(
|
||||
"Line number to comment on. For multi-line ranges, this is the end line. Use NEW column from diff format."
|
||||
"Line number to comment on. For multi-line ranges, this is the end line. Use NEW column from diff format. MUST sit inside a `@@` hunk in the PR diff — anchors on context-only or untouched lines are dropped silently (the rest of the review still posts; dropped entries are reported under `droppedComments` in the response)."
|
||||
),
|
||||
side: type
|
||||
.enumerated("LEFT", "RIGHT")
|
||||
@@ -345,7 +347,7 @@ export const CreatePullRequestReview = type({
|
||||
.optional(),
|
||||
start_line: type.number
|
||||
.describe(
|
||||
"Start line for multi-line comment ranges. Omit for single-line comments. The range [start_line, line] defines which lines a suggestion replaces."
|
||||
"Start line for multi-line comment ranges. Omit for single-line comments. The range [start_line, line] defines which lines a suggestion replaces. BOTH `start_line` AND `line` must sit inside the SAME `@@` hunk — a `start_line` outside the hunk causes the whole comment to be dropped even when `line` is valid. If you need to comment on context just above/below a hunk, shrink the range to a single line that is provably modified."
|
||||
)
|
||||
.optional(),
|
||||
})
|
||||
|
||||
+3
-1
@@ -15,7 +15,9 @@ import { execute, tool } from "./shared.ts";
|
||||
export const ShellParams = type({
|
||||
command: "string",
|
||||
description: "string",
|
||||
"timeout?": "number",
|
||||
"timeout?": type.number.describe(
|
||||
"Timeout in MILLISECONDS (not seconds). Default 30000 (30s), max 120000 (2m). e.g. timeout: 180000 for 3 minutes; timeout: 180 means 180ms and will kill the process almost immediately."
|
||||
),
|
||||
"working_directory?": "string",
|
||||
"background?": "boolean",
|
||||
});
|
||||
|
||||
+53
-10
@@ -4,6 +4,7 @@ import { encode as toonEncode } from "@toon-format/toon";
|
||||
import { type AgentId, formatMcpToolRef, type PayloadEvent, pullfrogMcpName } from "../external.ts";
|
||||
import type { Mode } from "../modes.ts";
|
||||
import type { ResolvedPayload } from "./payload.ts";
|
||||
import type { LearningsHeading } from "./runContext.ts";
|
||||
import type { RunContextData } from "./runContextData.ts";
|
||||
|
||||
interface InstructionsContext {
|
||||
@@ -16,6 +17,10 @@ interface InstructionsContext {
|
||||
* couldn't be seeded for some reason. main.ts always seeds, so in
|
||||
* practice this is always set; the null case keeps the type honest. */
|
||||
learningsFilePath: string | null;
|
||||
/** server-parsed TOC for the body of the learnings tmpfile. rendered
|
||||
* inline into the LEARNINGS prompt section so the agent can `read_file`
|
||||
* targeted line ranges instead of pulling the whole file into context. */
|
||||
learningsHeadings: LearningsHeading[];
|
||||
}
|
||||
|
||||
interface PromptContext extends InstructionsContext {
|
||||
@@ -369,6 +374,39 @@ export interface ResolvedInstructions {
|
||||
runtime: string;
|
||||
}
|
||||
|
||||
/** render the heading list as an indented bullet TOC. ranges shown in
|
||||
* parentheses (`(L3-L18)`); the start line is always the heading line
|
||||
* itself, so reading the listed range gives the agent the heading +
|
||||
* body together. shallowest heading depth in the body sits at the root
|
||||
* column; deeper levels indent by `(depth - rootDepth) * 2` spaces. */
|
||||
export function renderLearningsToc(headings: LearningsHeading[]): string {
|
||||
if (headings.length === 0) return "";
|
||||
const rootDepth = Math.min(...headings.map((h) => h.depth));
|
||||
return headings
|
||||
.map((h) => {
|
||||
const indent = " ".repeat((h.depth - rootDepth) * 2);
|
||||
return `${indent}- ${h.title} (L${h.startLine}-L${h.endLine})`;
|
||||
})
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
/** assemble the LEARNINGS prompt section: file path + intro + either
|
||||
* the rendered heading TOC (when the body has structure) or a no-headings
|
||||
* affordance pointing the agent at the reflection turn for restructuring.
|
||||
* empty string when the seed step failed and there's no path to surface. */
|
||||
export function buildLearningsSection(ctx: {
|
||||
filePath: string | null;
|
||||
headings: LearningsHeading[];
|
||||
}): string {
|
||||
if (!ctx.filePath) return "";
|
||||
const intro = `Repo-level learnings accumulated by previous agent runs live at \`${ctx.filePath}\`. Use this file as durable context (test commands, conventions, gotchas, architecture notes).`;
|
||||
const tocBody =
|
||||
ctx.headings.length === 0
|
||||
? "(no headings yet — file is empty or a flat list. read the whole file. during the post-run reflection turn, structure it with `## ` / `### ` headings so future runs can read targeted ranges.)"
|
||||
: `Read targeted line ranges via your native file tool — do NOT slurp the whole file. Each range starts at the section heading line, so reading the range gives you heading + body together.\n\n${renderLearningsToc(ctx.headings)}`;
|
||||
return `************* LEARNINGS *************\n\n${intro}\n\n${tocBody}`;
|
||||
}
|
||||
|
||||
function assembleFullPrompt(ctx: {
|
||||
toc: string;
|
||||
task: string;
|
||||
@@ -376,17 +414,18 @@ function assembleFullPrompt(ctx: {
|
||||
eventContext: string;
|
||||
system: string;
|
||||
learningsFilePath: string | null;
|
||||
learningsHeadings: LearningsHeading[];
|
||||
runtime: string;
|
||||
}): string {
|
||||
// the LEARNINGS section is intentionally tiny — just the file path and a
|
||||
// one-line "read it" instruction. embedding the contents would re-inflate
|
||||
// the prompt every run (the previous design's failure mode) and clutter
|
||||
// CI logs. the agent reads the file with its native file tool; the
|
||||
// post-run reflection turn (action/agents/postRun.ts) is where editing
|
||||
// is encouraged, with the prune-stale framing.
|
||||
const learningsSection = ctx.learningsFilePath
|
||||
? `************* LEARNINGS *************\n\nRepo-level learnings accumulated by previous agent runs live at \`${ctx.learningsFilePath}\`. Read this file early and let the entries inform your approach (test commands, conventions, gotchas, etc.). The file may be empty if no learnings have been collected yet.`
|
||||
: "";
|
||||
// server-parsed TOC is rendered inline so the agent can target line
|
||||
// ranges via its native file tool. the file body itself is never
|
||||
// inlined — that would re-inflate context every run and clutter CI
|
||||
// logs. post-run reflection (action/agents/postRun.ts) is where
|
||||
// editing is encouraged.
|
||||
const learningsSection = buildLearningsSection({
|
||||
filePath: ctx.learningsFilePath,
|
||||
headings: ctx.learningsHeadings,
|
||||
});
|
||||
|
||||
const runtimeSection = `************* RUNTIME *************\n\n${ctx.runtime}`;
|
||||
|
||||
@@ -421,7 +460,10 @@ export function resolveInstructions(ctx: InstructionsContext): ResolvedInstructi
|
||||
tocEntries.push({ label: "EVENT CONTEXT", description: "related PR/issue data" });
|
||||
tocEntries.push({ label: "SYSTEM", description: "persona, security, tools, workflow rules" });
|
||||
if (pctx.learningsFilePath)
|
||||
tocEntries.push({ label: "LEARNINGS", description: "repo-specific knowledge file path" });
|
||||
tocEntries.push({
|
||||
label: "LEARNINGS",
|
||||
description: "repo-specific knowledge file path + heading TOC",
|
||||
});
|
||||
tocEntries.push({ label: "RUNTIME", description: "environment metadata" });
|
||||
|
||||
const toc = buildToc(tocEntries);
|
||||
@@ -433,6 +475,7 @@ export function resolveInstructions(ctx: InstructionsContext): ResolvedInstructi
|
||||
eventContext,
|
||||
system,
|
||||
learningsFilePath: pctx.learningsFilePath,
|
||||
learningsHeadings: pctx.learningsHeadings,
|
||||
runtime: pctx.runtime,
|
||||
});
|
||||
|
||||
|
||||
+45
-28
@@ -1,4 +1,4 @@
|
||||
import { mkdtemp, rm, writeFile } from "node:fs/promises";
|
||||
import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
@@ -20,51 +20,68 @@ describe("learnings tmpfile round-trip", () => {
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("seeds with existing learnings and reads them back verbatim", async () => {
|
||||
const current = "- run tests with `pnpm -r test`\n- default branch is `main`";
|
||||
it("writes the verbatim DB body to disk and reads it back unchanged", async () => {
|
||||
const current = [
|
||||
"## Build & test",
|
||||
"",
|
||||
"- run tests with `pnpm -r test`",
|
||||
"",
|
||||
"## Architecture",
|
||||
"",
|
||||
"- workers in `worker/`",
|
||||
].join("\n");
|
||||
const path = await seedLearningsFile({ tmpdir: dir, current });
|
||||
expect(path).toBe(learningsFilePath(dir));
|
||||
expect(path.endsWith(LEARNINGS_FILE_NAME)).toBe(true);
|
||||
const read = await readLearningsFile(path);
|
||||
expect(read).toBe(current);
|
||||
|
||||
expect(await readFile(path, "utf8")).toBe(current);
|
||||
expect(await readLearningsFile(path)).toBe(current);
|
||||
});
|
||||
|
||||
it("seeds an empty file when the repo has no learnings yet", async () => {
|
||||
// empty seed (vs scaffold-with-comment) keeps the byte-trim equality
|
||||
// gate clean: an untouched first run reads back as "" and persistLearnings
|
||||
// skips the API round-trip rather than writing a placeholder string into
|
||||
// Repo.learnings.
|
||||
it("seeds an empty file when the repo has no learnings yet, round-trip is empty string", async () => {
|
||||
const path = await seedLearningsFile({ tmpdir: dir, current: null });
|
||||
const read = await readLearningsFile(path);
|
||||
expect(read).toBe("");
|
||||
expect(await readFile(path, "utf8")).toBe("");
|
||||
expect(await readLearningsFile(path)).toBe("");
|
||||
});
|
||||
|
||||
it("returns null when the file is missing (treated as no-change by persist)", async () => {
|
||||
const path = learningsFilePath(dir);
|
||||
const read = await readLearningsFile(path);
|
||||
expect(read).toBeNull();
|
||||
expect(await readLearningsFile(learningsFilePath(dir))).toBeNull();
|
||||
});
|
||||
|
||||
it("trims whitespace so trailing newlines never trigger a spurious PATCH", async () => {
|
||||
// editors commonly add a trailing newline on save. without trimming, a
|
||||
// round-trip "read seed → save unchanged" would fail byte-equality and
|
||||
// burn a LearningsRevision row on every run.
|
||||
const current = "- one fact";
|
||||
it("trims trailing whitespace so editor newlines never trigger a spurious PATCH", async () => {
|
||||
const current = "## Build & test\n\n- one fact";
|
||||
const path = await seedLearningsFile({ tmpdir: dir, current });
|
||||
await writeFile(path, `${current}\n\n `, "utf8");
|
||||
const read = await readLearningsFile(path);
|
||||
expect(read).toBe(current);
|
||||
expect(await readLearningsFile(path)).toBe(current);
|
||||
});
|
||||
|
||||
it("truncates content over the 10k server-side cap", async () => {
|
||||
// server enforces MAX_LEARNINGS_LENGTH = 10_000. truncating client-side
|
||||
// avoids a 400 round-trip and keeps the bytes the agent will see in the
|
||||
// next run aligned with what the server actually stored.
|
||||
const oversized = "x".repeat(11_000);
|
||||
it("truncates over-cap bodies at the last newline boundary so the next-seed TOC parse stays clean", async () => {
|
||||
const padding = `${"x".repeat(80)}\n`.repeat(1300);
|
||||
const oversized = `## Build & test\n\n${padding}`;
|
||||
const path = await seedLearningsFile({ tmpdir: dir, current: null });
|
||||
await writeFile(path, oversized, "utf8");
|
||||
const read = await readLearningsFile(path);
|
||||
expect(read).toBeTruthy();
|
||||
expect(read?.length).toBe(10_000);
|
||||
expect(read?.length).toBeLessThanOrEqual(100_000);
|
||||
const tailLine = read?.split("\n").pop() ?? "";
|
||||
expect(/^x+$/.test(tailLine)).toBe(true);
|
||||
expect(tailLine.length).toBe(80);
|
||||
});
|
||||
|
||||
it("falls back to a hard truncate when the only newline is far above the cap (giant single line)", async () => {
|
||||
const oversized = `## Build & test\n${"x".repeat(110_000)}`;
|
||||
const path = await seedLearningsFile({ tmpdir: dir, current: null });
|
||||
await writeFile(path, oversized, "utf8");
|
||||
const read = await readLearningsFile(path);
|
||||
expect(read).toBeTruthy();
|
||||
expect(read?.length).toBe(100_000);
|
||||
expect(read?.startsWith("## Build & test\n")).toBe(true);
|
||||
});
|
||||
|
||||
it("preserves legacy free-text without scaffolding or wrapping", async () => {
|
||||
const legacy = "- this is some old free-text bullet\n- another one";
|
||||
const path = await seedLearningsFile({ tmpdir: dir, current: legacy });
|
||||
expect(await readFile(path, "utf8")).toBe(legacy);
|
||||
expect(await readLearningsFile(path)).toBe(legacy);
|
||||
});
|
||||
});
|
||||
|
||||
+44
-18
@@ -7,47 +7,75 @@ import { dirname, join } from "node:path";
|
||||
* back into future runs as durable context. Modeled on the PR-summary tmpfile
|
||||
* pattern (see action/utils/prSummary.ts):
|
||||
*
|
||||
* 1. server seeds `pullfrog-learnings.md` from `Repo.learnings` (or empty
|
||||
* when the repo has none yet)
|
||||
* 2. the agent reads the file at startup as part of its context, and may
|
||||
* edit it in place at end-of-run when prompted by the reflection turn
|
||||
* 3. main.ts reads the file back at end-of-run and PATCHes
|
||||
* `/api/repo/[owner]/[repo]/learnings` if it changed (byte-trim equality
|
||||
* against the seed determines change detection)
|
||||
* 1. server seeds `pullfrog-learnings.md` with the verbatim body of
|
||||
* `Repo.learnings` (or empty for fresh repos), and parses headings
|
||||
* server-side (`utils/learningsToc.ts`) — the parsed TOC is rendered
|
||||
* into the LEARNINGS prompt section, not into the file
|
||||
* 2. the agent reads the TOC in the prompt and uses listed line ranges
|
||||
* to read just the sections relevant to the current task — file can
|
||||
* grow large, but only targeted ranges hit the agent's context
|
||||
* 3. agent edits the file in place at end-of-run during the reflection
|
||||
* turn (see action/agents/postRun.ts buildLearningsReflectionPrompt)
|
||||
* 4. main.ts reads the file back at end-of-run and PATCHes
|
||||
* `/api/repo/[owner]/[repo]/learnings` if the body changed
|
||||
*
|
||||
* Edit-in-place avoids stuffing the entire learnings list into both the
|
||||
* prompt context and an `update_learnings` MCP tool call (which previously
|
||||
* required passing the FULL merged list as a string parameter — an
|
||||
* output-token tax that grew linearly with the learnings size).
|
||||
*
|
||||
* Section structure is agent-curated. The reflection prompt teaches
|
||||
* hierarchy + a soft 300-line-per-section cap to keep TOC ranges
|
||||
* agent-targetable on long-lived repos; there is no fixed taxonomy.
|
||||
*/
|
||||
|
||||
export const LEARNINGS_FILE_NAME = "pullfrog-learnings.md";
|
||||
|
||||
/** server-side cap mirrors `MAX_LEARNINGS_LENGTH` in
|
||||
* `app/api/repo/[owner]/[repo]/learnings/route.ts`. truncating client-side
|
||||
* keeps the PATCH from being rejected with a 400. */
|
||||
const MAX_LEARNINGS_LENGTH = 10_000;
|
||||
* keeps the PATCH from being rejected with a 400. raised from 10k → 100k
|
||||
* once the TOC affordance landed: with line-range reads via the
|
||||
* server-parsed TOC the agent doesn't ingest the whole file, so the cap
|
||||
* can grow to whatever curation discipline allows. 100k holds ~400-500
|
||||
* short bullets. */
|
||||
const MAX_LEARNINGS_LENGTH = 100_000;
|
||||
|
||||
export function learningsFilePath(tmpdir: string): string {
|
||||
return join(tmpdir, LEARNINGS_FILE_NAME);
|
||||
}
|
||||
|
||||
/** seed the learnings file with the repo's current learnings, or an empty
|
||||
* file when the repo has none yet. returns the absolute path. */
|
||||
/** seed the rolling learnings tmpfile with the verbatim DB body (or empty
|
||||
* string for fresh repos). returns the absolute path. the parsed TOC is
|
||||
* carried separately via `RepoSettings.learningsHeadings` and rendered
|
||||
* into the prompt by `resolveInstructions`, so the file on disk is just
|
||||
* the body — no markers, no scaffold, no in-file TOC. */
|
||||
export async function seedLearningsFile(params: {
|
||||
tmpdir: string;
|
||||
current: string | null;
|
||||
}): Promise<string> {
|
||||
const path = learningsFilePath(params.tmpdir);
|
||||
await mkdir(dirname(path), { recursive: true });
|
||||
// empty file when no learnings exist yet — the agent reads it, sees
|
||||
// nothing, and the LEARNINGS prompt section explains what the file is for.
|
||||
// a header comment would risk being persisted as part of the first real
|
||||
// edit, polluting the DB row with placeholder text.
|
||||
await writeFile(path, params.current ?? "", "utf8");
|
||||
return path;
|
||||
}
|
||||
|
||||
/** truncate at the last newline boundary before `cap` so we don't leave
|
||||
* a partial line at the tail (a half-truncated `## Headi` confuses the
|
||||
* server's next-seed TOC parse and shrinks visible structure). falls
|
||||
* back to a hard `slice` when the line boundary would discard a large
|
||||
* run of content — i.e. when the tail of `head` is one giant line (rare:
|
||||
* minified pastes, fenced log dumps). losing a partial last line is
|
||||
* preferable to losing kilobytes of body. */
|
||||
const TRUNCATION_LINE_BOUNDARY_TOLERANCE = 4096;
|
||||
function truncateAtLineBoundary(body: string, cap: number): string {
|
||||
if (body.length <= cap) return body;
|
||||
const head = body.slice(0, cap);
|
||||
const lastNewline = head.lastIndexOf("\n");
|
||||
if (lastNewline <= 0) return head;
|
||||
if (cap - lastNewline > TRUNCATION_LINE_BOUNDARY_TOLERANCE) return head;
|
||||
return head.slice(0, lastNewline);
|
||||
}
|
||||
|
||||
/** read the agent-edited learnings file. returns null when the file is
|
||||
* missing or unreadable (treated as "no change"). caps content at the
|
||||
* server's max length to avoid a 400 round-trip. */
|
||||
@@ -58,7 +86,5 @@ export async function readLearningsFile(path: string): Promise<string | null> {
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
const trimmed = raw.trim();
|
||||
if (trimmed.length > MAX_LEARNINGS_LENGTH) return trimmed.slice(0, MAX_LEARNINGS_LENGTH);
|
||||
return trimmed;
|
||||
return truncateAtLineBoundary(raw.trim(), MAX_LEARNINGS_LENGTH);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { buildLearningsSection, renderLearningsToc } from "./instructions.ts";
|
||||
import type { LearningsHeading } from "./runContext.ts";
|
||||
|
||||
const h = (depth: 1 | 2 | 3 | 4 | 5 | 6, title: string, startLine: number, endLine: number) => ({
|
||||
depth,
|
||||
title,
|
||||
startLine,
|
||||
endLine,
|
||||
});
|
||||
|
||||
describe("renderLearningsToc", () => {
|
||||
it("renders flat h2 list with parenthesized ranges, no hashes or backticks", () => {
|
||||
const headings: LearningsHeading[] = [
|
||||
h(2, "Build & test", 1, 18),
|
||||
h(2, "Architecture", 19, 60),
|
||||
];
|
||||
expect(renderLearningsToc(headings)).toBe(
|
||||
`- Build & test (L1-L18)
|
||||
- Architecture (L19-L60)`
|
||||
);
|
||||
});
|
||||
|
||||
it("indents deeper headings 2 spaces per depth level past the shallowest", () => {
|
||||
const headings: LearningsHeading[] = [
|
||||
h(2, "Build & test", 1, 42),
|
||||
h(3, "Local", 3, 18),
|
||||
h(3, "CI", 19, 42),
|
||||
h(2, "Architecture", 43, 210),
|
||||
h(3, "Background workers", 80, 210),
|
||||
];
|
||||
expect(renderLearningsToc(headings)).toBe(
|
||||
`- Build & test (L1-L42)
|
||||
- Local (L3-L18)
|
||||
- CI (L19-L42)
|
||||
- Architecture (L43-L210)
|
||||
- Background workers (L80-L210)`
|
||||
);
|
||||
});
|
||||
|
||||
it("treats the shallowest depth as the root column when no h2 is present", () => {
|
||||
const headings: LearningsHeading[] = [h(3, "Only h3", 1, 5), h(4, "Sub h4", 2, 5)];
|
||||
expect(renderLearningsToc(headings)).toBe(
|
||||
`- Only h3 (L1-L5)
|
||||
- Sub h4 (L2-L5)`
|
||||
);
|
||||
});
|
||||
|
||||
it("supports depths up through h6 with stable 2-space indent steps", () => {
|
||||
const headings: LearningsHeading[] = [
|
||||
h(2, "Two", 1, 10),
|
||||
h(3, "Three", 2, 10),
|
||||
h(4, "Four", 3, 10),
|
||||
h(5, "Five", 4, 10),
|
||||
h(6, "Six", 5, 10),
|
||||
];
|
||||
expect(renderLearningsToc(headings)).toBe(
|
||||
`- Two (L1-L10)
|
||||
- Three (L2-L10)
|
||||
- Four (L3-L10)
|
||||
- Five (L4-L10)
|
||||
- Six (L5-L10)`
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildLearningsSection", () => {
|
||||
it("returns empty string when no file path (seed step failed)", () => {
|
||||
expect(buildLearningsSection({ filePath: null, headings: [] })).toBe("");
|
||||
});
|
||||
|
||||
it("renders the no-headings affordance when the body has no structure", () => {
|
||||
const out = buildLearningsSection({
|
||||
filePath: "/tmp/run-1/pullfrog-learnings.md",
|
||||
headings: [],
|
||||
});
|
||||
expect(out).toContain("************* LEARNINGS *************");
|
||||
expect(out).toContain("/tmp/run-1/pullfrog-learnings.md");
|
||||
expect(out).toContain("no headings yet");
|
||||
expect(out).toContain("structure it with");
|
||||
// does not include a TOC list when there are no headings
|
||||
expect(out).not.toMatch(/\(L\d+-L\d+\)/);
|
||||
});
|
||||
|
||||
it("renders the TOC inline with the file path and heading guidance", () => {
|
||||
const out = buildLearningsSection({
|
||||
filePath: "/tmp/run-1/pullfrog-learnings.md",
|
||||
headings: [h(2, "Build & test", 1, 18), h(2, "Architecture", 19, 60)],
|
||||
});
|
||||
expect(out).toContain("************* LEARNINGS *************");
|
||||
expect(out).toContain("/tmp/run-1/pullfrog-learnings.md");
|
||||
expect(out).toContain("- Build & test (L1-L18)");
|
||||
expect(out).toContain("- Architecture (L19-L60)");
|
||||
expect(out).toContain("Each range starts at the section heading line");
|
||||
// explicit "no hashes, no backticks" in the rendered list
|
||||
expect(out).not.toContain("- `## Build");
|
||||
expect(out).not.toContain("`## Build");
|
||||
});
|
||||
});
|
||||
@@ -9,6 +9,22 @@ export interface Mode {
|
||||
prompt: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* server-parsed TOC entry for `Repo.learnings`. depth is 1-6 (h1-h6),
|
||||
* line numbers are 1-indexed against the raw body. computed by
|
||||
* `parseLearningsHeadings` in `utils/learningsToc.ts` (server side) and
|
||||
* shipped over the run-context JSON boundary; the canonical declaration
|
||||
* lives there. duplicated here because the action runtime can't reach
|
||||
* across into the proprietary root-level codebase, and the JSON wire
|
||||
* means typecheck can't enforce shape equality across both sides.
|
||||
*/
|
||||
export interface LearningsHeading {
|
||||
depth: 1 | 2 | 3 | 4 | 5 | 6;
|
||||
title: string;
|
||||
startLine: number;
|
||||
endLine: number;
|
||||
}
|
||||
|
||||
export interface RepoSettings {
|
||||
model: string | null;
|
||||
modes: Mode[];
|
||||
@@ -21,6 +37,7 @@ export interface RepoSettings {
|
||||
prApproveEnabled: boolean;
|
||||
modeInstructions: Record<string, string>;
|
||||
learnings: string | null;
|
||||
learningsHeadings: LearningsHeading[];
|
||||
envAllowlist: string | null;
|
||||
}
|
||||
|
||||
@@ -61,6 +78,7 @@ const defaultSettings: RepoSettings = {
|
||||
prApproveEnabled: false,
|
||||
modeInstructions: {},
|
||||
learnings: null,
|
||||
learningsHeadings: [],
|
||||
envAllowlist: null,
|
||||
};
|
||||
|
||||
@@ -127,6 +145,7 @@ export async function fetchRunContext(params: {
|
||||
postCheckoutScript: data.settings?.postCheckoutScript ?? null,
|
||||
prepushScript: data.settings?.prepushScript ?? null,
|
||||
stopScript: data.settings?.stopScript ?? null,
|
||||
learningsHeadings: data.settings?.learningsHeadings ?? [],
|
||||
},
|
||||
apiToken: data.apiToken,
|
||||
oss: data.oss ?? false,
|
||||
|
||||
Reference in New Issue
Block a user