Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
codex_autofix_changed: ${{ steps.scope.outputs.codex_autofix_changed }}
build_changed: ${{ steps.scope.outputs.build_changed }}
lockfile_changed: ${{ steps.scope.outputs.lockfile_changed }}
pr_policy_body_present: ${{ steps.scope.outputs.pr_policy_body_present }}
pr_policy_body_changed: ${{ steps.scope.outputs.pr_policy_body_changed }}
steps:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
Expand Down Expand Up @@ -88,7 +88,7 @@ jobs:
sync-pr-policy-body:
name: Sync PR policy body
needs: changes
if: github.event_name == 'pull_request' && needs.changes.outputs.pr_policy_body_present == 'true'
if: github.event_name == 'pull_request' && needs.changes.outputs.pr_policy_body_changed == 'true'
runs-on: ubuntu-24.04
timeout-minutes: 5
permissions:
Expand Down
1 change: 1 addition & 0 deletions docs/branch-review-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -899,3 +899,4 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie
| 2026-08-11 | claude/spacing-icon-design-review-rxwh28 | 5b96281ee7da817d5ce7f1102004ebe6f861b920 | pr-1815 heavy review-and-fix | remote already merged main (shadow-tight Switch kept); cherry-picked privacy -mb-4 reclaim + calculators dock cancel; removed duplicate UniversalSearchAlsoMatches; rail-aware section-sheet focus restore; dispositioned CodeRabbit docs/ledger/gates nits and outdated Sentry skeleton gap | verify:cheap PASS prior tip; verify:pr-local PASS prior tip; vitest privacy+in-page-nav 28 passed on cherry-pick; merge-tree clean vs origin/main |
| 2026-08-12 | PR #1815 / claude/spacing-icon-design-review-rxwh28 | 9f266210f02081be54d407c70a85f52fed436128 | babysit | no remaining actionable findings; one pre-existing thread resolved as no-change (Dockerfile.worker follow-up needed) | required checks: Gitleaks PR policy PR required (all pass); targeted vitest passed: tests/document-frame-contract.test.ts + tests/in-page-nav-header.dom.test.tsx |
| 2026-08-12 | 1815 | 27ce96e1755055ceee2eeae02d6efdf11259fcde | babysit | fixed | Unit coverage: targeted vitest passed: tests/shared-home-empty-state.dom.test.tsx (17 passed). PR required still blocked on pre-existing check failure at old remote head before sync. |
| 2026-08-12 | codex/pr-workflow-safety-230-296 | bc0a491fdf4146775629f9b2b03e2a2cc61bd7cb | pr-1830 unblock | unblocked: merged origin/main (outstanding-issues conflict), PR body RAG impact + governance, resolved Copilot thread; merge-tree clean; required CI in progress | check:outstanding-issues pass; evaluatePullRequestPolicy ok; merge-tree clean; PR policy/mergeability/Change scope in progress |
235 changes: 117 additions & 118 deletions docs/outstanding-issues.md

Large diffs are not rendered by default.

21 changes: 11 additions & 10 deletions scripts/ci-change-scope.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env node
import { execFileSync } from "node:child_process";
import { appendFileSync, existsSync, readFileSync } from "node:fs";
import { appendFileSync, readFileSync } from "node:fs";

const zeroSha = /^0{40}$/;

Expand Down Expand Up @@ -34,7 +34,7 @@ const outputs = [
"codex_autofix_changed",
"build_changed",
"lockfile_changed",
"pr_policy_body_present",
"pr_policy_body_changed",
];

function normalizePath(filePath) {
Expand Down Expand Up @@ -362,7 +362,7 @@ function isRecognisedLightPath(filePath) {
// drive both the empty and non-empty cases without touching the real file.
const readFlakeLedger = () => readFileSync("tests/flake-ledger.json", "utf8");

function classify(files, { readLedger = readFlakeLedger, prPolicyBodyPresent = existsSync("PR_POLICY_BODY.md") } = {}) {
function classify(files, { readLedger = readFlakeLedger } = {}) {
const normalized = [...new Set(files.map(normalizePath).filter(Boolean))].sort();
const docsChanged = normalized.some((file) => pathMatches(file, docPatterns));
const sourceChanged = normalized.some((file) => pathMatches(file, [...sourcePatterns, ...staticConfigPatterns]));
Expand All @@ -376,6 +376,7 @@ function classify(files, { readLedger = readFlakeLedger, prPolicyBodyPresent = e
const workflowChanged = normalized.some((file) => pathMatches(file, workflowPatterns));
const codexAutofixChanged = normalized.some((file) => pathMatches(file, codexAutofixPatterns));
const lockfileChanged = normalized.some((file) => pathMatches(file, lockfilePatterns));
const prPolicyBodyChanged = normalized.includes("PR_POLICY_BODY.md");
const buildChanged = normalized.some((file) => pathMatches(file, buildPatterns)) || containerChanged;
// Only two categories are allowed to take the lightweight path: recognised
// documentation and recognised non-executable workflow/policy surfaces.
Expand Down Expand Up @@ -415,7 +416,7 @@ function classify(files, { readLedger = readFlakeLedger, prPolicyBodyPresent = e
codex_autofix_changed: codexAutofixChanged,
build_changed: buildChanged,
lockfile_changed: lockfileChanged,
pr_policy_body_present: prPolicyBodyPresent,
pr_policy_body_changed: prPolicyBodyChanged,
};
}

Expand Down Expand Up @@ -1172,12 +1173,12 @@ function selfTest() {
static_heavy_changed: true,
coverage_changed: true,
});
assertScope(
"pr-policy-body-presence-is-routed-without-a-second-checkout",
["PR_POLICY_BODY.md"],
{ pr_policy_body_present: true },
{ prPolicyBodyPresent: true },
);
assertScope("pr-policy-body-change-is-routed-from-the-pr-diff", ["PR_POLICY_BODY.md"], {
pr_policy_body_changed: true,
});
assertScope("inherited-pr-policy-body-does-not-sync", ["docs/testing.md"], {
pr_policy_body_changed: false,
});
console.log("CI change scope self-test passed.");
}

Expand Down
12 changes: 12 additions & 0 deletions src/lib/rag/rag-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,14 @@ function sharedCacheSelector(
return query;
}

const GENERATION_FALLBACK_MARKER = /(?:^|;\s*)generation_fallback(?::|$)/i;
function isGenerationFallbackAnswer(answer: Pick<RagAnswer, "routingReason" | "degradedMode">) {
return (
GENERATION_FALLBACK_MARKER.test(answer.routingReason ?? "") ||
GENERATION_FALLBACK_MARKER.test(answer.degradedMode?.reason ?? "")
);
Comment thread
BigSimmo marked this conversation as resolved.
}

export async function cacheIndexingVersion(
args: Pick<SearchChunksArgs, "documentId" | "documentIds" | "ownerId" | "accessScope" | "signal">,
options?: { forceRefresh?: boolean },
Expand Down Expand Up @@ -613,6 +621,10 @@ export async function getSharedCachedAnswer(
).maybeSingle();
if (error || !data?.payload) return null;
const answer = cloneAnswer((data.payload as { answer: RagAnswer }).answer);
if (isGenerationFallbackAnswer(answer)) {
await deleteSharedCachedAnswerRow({ ...args, accessScope: retrievalAccessScopeForArgs(args) }, indexingVersion);
return null;
}
answer.routingReason = answer.routingReason
? `${answer.routingReason}; shared_answer_cache_hit`
: "shared_answer_cache_hit";
Expand Down
18 changes: 16 additions & 2 deletions src/lib/rag/rag-route-budget.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { AnswerRouteMode } from "@/lib/rag/rag-routing";
import type { RagAnswer } from "@/lib/types";

export const answerRouteBudgetMs = {
unsupported: 0,
Expand All @@ -17,6 +18,12 @@ export const generationRecoveryReserveMs = 2_000;
// spends MORE reasoning under a boosted cap, so anything shorter is a guaranteed-discard.
export const minimumGenerationRetryMs = 5_000;

// Keep in lockstep with rag-cache / isProviderGenerationDegraded: bare
// `generation_fallback` (no `:reason`) and case variants must refuse the cache.
// Inline the marker here so this leaf module does not import rag-answer-support
// (that path pulls env through deep-memory and freezes the offline vitest snapshot).
const GENERATION_FALLBACK_MARKER = /(?:^|;\s*)generation_fallback(?::|$)/i;

export class AnswerRouteDeadlineExceededError extends Error {
readonly routeMode: AnswerRouteMode;
readonly budgetMs: number;
Expand Down Expand Up @@ -49,8 +56,15 @@ export function deadlineAllowsGenerationRetry(deadline: Pick<AnswerRouteDeadline
return deadline.remainingMs() >= generationRecoveryReserveMs + minimumGenerationRetryMs;
}

export function answerRouteResultCanBeCached(deadline: Pick<AnswerRouteDeadline, "deadlineExceeded">) {
return !deadline.deadlineExceeded;
export function answerRouteResultCanBeCached(
deadline: Pick<AnswerRouteDeadline, "deadlineExceeded">,
answer: Pick<RagAnswer, "routingReason" | "degradedMode">,
) {
return (
!deadline.deadlineExceeded &&
!GENERATION_FALLBACK_MARKER.test(answer.routingReason ?? "") &&
!GENERATION_FALLBACK_MARKER.test(answer.degradedMode?.reason ?? "")
);
}

function abortReason(signal: AbortSignal) {
Expand Down
25 changes: 4 additions & 21 deletions src/lib/rag/rag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2413,23 +2413,6 @@ function buildContextDerivedArtifacts(query: string, results: SearchResult[]) {
};
}

export function isCacheableGroundedGenerationFallback(
answer: Pick<
RagAnswer,
"routingMode" | "routingReason" | "grounded" | "confidence" | "citations" | "unverifiedNumericTokens"
>,
) {
return (
answer.routingMode === "extractive" &&
answer.grounded &&
answer.confidence !== "unsupported" &&
answer.citations.length > 0 &&
(answer.unverifiedNumericTokens?.length ?? 0) === 0 &&
/(?:source_backed_extractive_fallback|comparison_source_safe_fallback)/.test(answer.routingReason ?? "") &&
!(answer.routingReason ?? "").includes(SOURCE_BACKED_REVIEW_FALLBACK_REASON)
);
}

/** Answer question. */
export async function answerQuestion(query: string, documentId?: string) {
return answerQuestionWithScope({ query, documentId, allowGlobalSearch: true });
Expand Down Expand Up @@ -2963,7 +2946,7 @@ async function answerQuestionWithScopeUncoalesced(

// Soft-tail unsupported refusals must not stick in the 5-minute answer cache.
if (
answerRouteResultCanBeCached(routeDeadline) &&
answerRouteResultCanBeCached(routeDeadline, finalizedAnswer) &&
!shouldSkipUnsupportedSoftTailAnswerCacheWrite({
resultCount: results.length,
retrievalStrategy: search.telemetry.retrieval_strategy,
Expand Down Expand Up @@ -3158,7 +3141,7 @@ async function answerQuestionWithScopeUncoalesced(
},
});

if (!routeDeadline.deadlineExceeded)
if (answerRouteResultCanBeCached(routeDeadline, finalizedAnswer))
await setCachedAnswer(args, finalizedAnswer, { indexingVersionAtRetrievalStart });
routeDeadline.dispose();
return finalizedAnswer;
Expand Down Expand Up @@ -3924,7 +3907,7 @@ ${qualityRetryInstruction}`
},
});

if (answerRouteResultCanBeCached(routeDeadline))
if (answerRouteResultCanBeCached(routeDeadline, answer))
await setCachedAnswer(args, answer, { indexingVersionAtRetrievalStart });
routeDeadline.dispose();
return answer;
Expand Down Expand Up @@ -4279,7 +4262,7 @@ ${qualityRetryInstruction}`
},
});

if (isCacheableGroundedGenerationFallback(fallbackAnswer) && !routeDeadline.deadlineExceeded) {
if (answerRouteResultCanBeCached(routeDeadline, fallbackAnswer)) {
await setCachedAnswer(args, fallbackAnswer, { indexingVersionAtRetrievalStart });
}
routeDeadline.dispose();
Expand Down
2 changes: 1 addition & 1 deletion tests/ci-cache-safety.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ describe("CI cache safety", () => {
expect(opsDigestWorkflow).toContain("eval-canary-liveness:");
expect(opsDigestWorkflow).toContain("github.rest.actions.listWorkflowRuns");
expect(workflow).toContain(
"if: github.event_name == 'pull_request' && needs.changes.outputs.pr_policy_body_present == 'true'",
"if: github.event_name == 'pull_request' && needs.changes.outputs.pr_policy_body_changed == 'true'",
);
});

Expand Down
38 changes: 18 additions & 20 deletions tests/pr-handoff-stop.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { execFileSync, spawnSync } from "node:child_process";
import { chmodSync, existsSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs";
import { existsSync, lstatSync, mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
Expand Down Expand Up @@ -162,26 +162,24 @@ describe("pr-handoff-stop hook", () => {

it("emits handoff context only when the marker file exists", () => {
const { root, gitDir } = freshRepo();
// Make the git dir unwritable so the marker write fails; post must fail
// open with no additionalContext (model must not be told tools are denied).
chmodSync(gitDir, 0o555);
// A directory at the exact marker path makes shell redirection fail for
// root and non-root users. Post must fail open with no additionalContext
// (the model must not be told tools are denied when no marker file landed).
const marker = join(gitDir, "claude-pr-handoff-sess-readonly");
mkdirSync(marker);

try {
const out = runHook(
"post",
{
tool_name: "create_pull_request",
session_id: "sess-readonly",
tool_response: "Opened https://github.com/BigSimmo/Database/pull/1649",
},
root,
);
expect(out.status).toBe(0);
expect(out.markerExists("sess-readonly")).toBe(false);
expect(out.stdout).toBe("");
} finally {
chmodSync(gitDir, 0o755);
}
const out = runHook(
"post",
{
tool_name: "create_pull_request",
session_id: "sess-readonly",
tool_response: "Opened https://github.com/BigSimmo/Database/pull/1649",
},
root,
);
expect(out.status).toBe(0);
expect(lstatSync(marker).isDirectory()).toBe(true);
expect(out.stdout).toBe("");
});

it("denies quoted compound follow commands when jq is unavailable", () => {
Expand Down
68 changes: 30 additions & 38 deletions tests/rag-answer-fallback.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { citationFromResult } from "../src/lib/citations";
import { answerRouteBudgetMs, generationRecoveryReserveMs } from "../src/lib/rag/rag-route-budget";
import {
answerRouteBudgetMs,
answerRouteResultCanBeCached,
generationRecoveryReserveMs,
} from "../src/lib/rag/rag-route-budget";
import type { RagAnswer, SearchResult } from "../src/lib/types";

function retrievalRpcBaseName(name: string) {
Expand Down Expand Up @@ -90,8 +94,13 @@ async function answerFromTextSources(
generatedAnswer?: GeneratedAnswerPayload | Error,
options: { sourceOnly?: boolean } = {},
) {
// `src/lib/env.ts` freezes process.env at module load. The offline vitest wrapper
// starts every worker as RAG_PROVIDER_MODE=offline with a blank OpenAI key, so we
// must re-parse env after stubbing — otherwise the first test in this file keeps
// the runner's offline snapshot and never exercises the mocked provider path.
vi.resetModules();
vi.stubEnv("OPENAI_API_KEY", options.sourceOnly ? "" : "test-key");
if (options.sourceOnly) vi.stubEnv("RAG_PROVIDER_MODE", "offline");
vi.stubEnv("RAG_PROVIDER_MODE", options.sourceOnly ? "offline" : "auto");
vi.stubEnv("RAG_SEARCH_CACHE_TTL_MS", "0");
vi.stubEnv("RAG_ANSWER_CACHE_TTL_MS", "0");

Expand Down Expand Up @@ -3387,7 +3396,7 @@ describe("RAG structured-output fallback", () => {
generateStructuredTextResult,
}));

const { answerQuestionWithScope, isCacheableGroundedGenerationFallback } = await import("../src/lib/rag/rag");
const { answerQuestionWithScope } = await import("../src/lib/rag/rag");
const progressEvents: Array<{
stage: string;
selectedContextCount?: number;
Expand Down Expand Up @@ -3430,7 +3439,7 @@ describe("RAG structured-output fallback", () => {
]);
expect(answer.openAIRequestIds).toEqual(["req_truncated_1", "req_truncated_2"]);
expect(answer.openAIUsage).toMatchObject({ output_tokens: 1300, total_tokens: 1500 });
expect(isCacheableGroundedGenerationFallback(answer)).toBe(true);
expect(answerRouteResultCanBeCached({ deadlineExceeded: false }, answer)).toBe(false);
expect(progressEvents).toContainEqual(
expect.objectContaining({
stage: "ranking",
Expand Down Expand Up @@ -3471,14 +3480,12 @@ describe("RAG structured-output fallback", () => {
],
new Error("OpenAI generation incomplete: max_output_tokens"),
);
const { isCacheableGroundedGenerationFallback } = await import("../src/lib/rag/rag");

expect(answer.answer).not.toMatch(/fluoxetine|citalopram|60 mg|40 mg/i);
expect(answer.answer).toMatch(/source|guidance|support|evidence/i);
expect(answer.routingReason).toContain("generation_fallback:provider_incomplete_max_output_tokens");
expect(answer.routingReason).toContain("source_backed_review_fallback");
expect(answer.unverifiedNumericTokens ?? []).toEqual([]);
expect(isCacheableGroundedGenerationFallback(answer)).toBe(false);
expect(answerRouteResultCanBeCached({ deadlineExceeded: false }, answer)).toBe(false);
});

it("prefers the safe single-chunk fallback candidate that carries the asked-for dose figure", async () => {
Expand Down Expand Up @@ -3523,40 +3530,25 @@ describe("RAG structured-output fallback", () => {
expect(new Set(answer.citations.map((citation) => citation.chunk_id))).toEqual(new Set(["quetiapine-maximum-1"]));
});

it("never marks the generic source-review fallback as cacheable", async () => {
const { isCacheableGroundedGenerationFallback } = await import("../src/lib/rag/rag");

it("never marks provider-generation fallbacks as cacheable", async () => {
expect(
isCacheableGroundedGenerationFallback({
routingMode: "unsupported",
routingReason: "strong_generation; generation_fallback:provider_timeout",
grounded: false,
confidence: "low",
citations: [],
unverifiedNumericTokens: [],
}),
answerRouteResultCanBeCached(
{ deadlineExceeded: false },
{
routingReason: "strong_generation; generation_fallback:provider_timeout",
degradedMode: { active: true, reason: "generation_fallback:provider_timeout" },
},
),
).toBe(false);
expect(
isCacheableGroundedGenerationFallback({
routingMode: "extractive",
routingReason:
"strong_generation; generation_fallback:provider_timeout; source_backed_review_fallback; extractive_quality_gate:weak",
grounded: true,
confidence: "low",
citations: [
{
chunk_id: "source-1",
document_id: "document-1",
title: "Source",
file_name: "source.pdf",
page_number: 1,
chunk_index: 0,
source_metadata: null,
provenance: "deterministic_support",
},
],
unverifiedNumericTokens: [],
}),
answerRouteResultCanBeCached(
{ deadlineExceeded: false },
{
routingReason:
"strong_generation; generation_fallback:provider_timeout; source_backed_review_fallback; extractive_quality_gate:weak",
degradedMode: { active: true, reason: "generation_fallback:provider_timeout" },
},
),
).toBe(false);
});

Expand Down
Loading
Loading