-
Notifications
You must be signed in to change notification settings - Fork 0
feat: preserve question dates and disclose CLI execution provenance #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import type { LanguageModel } from "ai" | ||
| import type { Judge, JudgeConfig, JudgeInput, JudgeResult } from "../types/judge" | ||
| import type { ProviderPrompts } from "../types/prompts" | ||
| import { buildJudgePrompt, parseJudgeResponse, getJudgePrompt } from "./base" | ||
| import { logger } from "../utils/logger" | ||
| import { CliCallError, cliComplete, cliLlmModelId, type CliCallTelemetry } from "../utils/cli-llm" | ||
|
|
||
| export class CliJudge implements Judge { | ||
| name = "cli" | ||
|
|
||
| async initialize(_config: JudgeConfig): Promise<void> { | ||
| logger.info(`Initialized CLI judge (${cliLlmModelId("judge")})`) | ||
| } | ||
|
|
||
| async evaluate(input: JudgeInput): Promise<JudgeResult> { | ||
| let execution: CliCallTelemetry | undefined | ||
| try { | ||
| const text = await cliComplete(buildJudgePrompt(input), { | ||
| role: "judge", | ||
| retry: false, | ||
| onTelemetry: (telemetry) => { | ||
| execution = telemetry | ||
| }, | ||
| }) | ||
| return { ...parseJudgeResponse(text), execution } | ||
| } catch (error) { | ||
| if (execution && !(error instanceof CliCallError)) { | ||
| throw new CliCallError(error instanceof Error ? error.message : String(error), execution) | ||
| } | ||
| throw error | ||
| } | ||
| } | ||
|
|
||
| getPromptForQuestionType(questionType: string, providerPrompts?: ProviderPrompts): string { | ||
| return getJudgePrompt(questionType, providerPrompts) | ||
| } | ||
|
|
||
| getModel(): LanguageModel { | ||
| throw new Error("CliJudge.getModel() is unavailable for CLI execution") | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,12 @@ | ||
| import type { ProviderName } from "../types/provider" | ||
| import type { BenchmarkName } from "../types/benchmark" | ||
| import type { JudgeName } from "../types/judge" | ||
| import type { RunCheckpoint, SamplingConfig } from "../types/checkpoint" | ||
| import type { LlmExecutionProvenance, RunCheckpoint, SamplingConfig } from "../types/checkpoint" | ||
| import type { ConcurrencyConfig } from "../types/concurrency" | ||
| import { createProvider } from "../providers" | ||
| import { createBenchmark } from "../benchmarks" | ||
| import { createJudge } from "../judges" | ||
| import { CliJudge } from "../judges/cli" | ||
| import { CheckpointManager } from "./checkpoint" | ||
| import { getProviderConfig, getJudgeConfig } from "../utils/config" | ||
| import { resolveModel } from "../utils/models" | ||
|
|
@@ -16,6 +17,8 @@ import { runSearchPhase } from "./phases/search" | |
| import { runAnswerPhase } from "./phases/answer" | ||
| import { runEvaluatePhase } from "./phases/evaluate" | ||
| import { generateReport, saveReport, printReport } from "./phases/report" | ||
| import { questionCheckpointMetadata, syncQuestionCheckpointMetadata } from "./question-metadata" | ||
| import { cliLlmBackend, cliLlmProvenance } from "../utils/cli-llm" | ||
|
|
||
| export interface OrchestratorOptions { | ||
| provider: ProviderName | ||
|
|
@@ -79,6 +82,21 @@ function selectQuestionsBySampling( | |
| return allQuestions.map((q) => q.questionId) | ||
| } | ||
|
|
||
| function resolveExecutionProvenance( | ||
| configuredModel: string, | ||
| role: "answerer" | "judge" | ||
| ): LlmExecutionProvenance { | ||
| const cli = cliLlmProvenance(role) | ||
| if (cli) return { ...cli, configuredModel, tokenizerModel: "gpt-4o" } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Hardcoded tokenizerModel=gpt-4o for every CLI backend
Category: Auth Why this matters: Provenance fields are meant to disclose exactly how a run was executed; an unconditional tokenizer label can mislead analysis comparing token economics across codex vs claude backends. |
||
| const resolved = resolveModel(configuredModel) | ||
| return { | ||
| transport: "ai-sdk", | ||
| model: resolved.id, | ||
| modelExplicit: true, | ||
| configuredModel, | ||
| } | ||
| } | ||
|
|
||
| export class Orchestrator { | ||
| private checkpointManager: CheckpointManager | ||
|
|
||
|
|
@@ -252,16 +270,29 @@ export class Orchestrator { | |
|
|
||
| for (const q of questionsToInit) { | ||
| const containerTag = `${q.questionId}-${checkpoint.dataSourceRunId}` | ||
| this.checkpointManager.initQuestion(checkpoint, q.questionId, containerTag, { | ||
| question: q.question, | ||
| groundTruth: q.groundTruth, | ||
| questionType: q.questionType, | ||
| }) | ||
| this.checkpointManager.initQuestion( | ||
| checkpoint, | ||
| q.questionId, | ||
| containerTag, | ||
| questionCheckpointMetadata(q) | ||
| ) | ||
| } | ||
|
|
||
| this.checkpointManager.updateStatus(checkpoint, "running") | ||
| } | ||
|
|
||
| const metadataUpdates = syncQuestionCheckpointMetadata(checkpoint, allQuestions) | ||
| if (metadataUpdates > 0) { | ||
| logger.info(`Backfilled questionDate for ${metadataUpdates} checkpoint questions`) | ||
| } | ||
| if (phases.includes("answer")) { | ||
| checkpoint.answererProvenance = resolveExecutionProvenance(answeringModel, "answerer") | ||
| } | ||
| if (phases.includes("evaluate")) { | ||
| checkpoint.judgeProvenance = resolveExecutionProvenance(judgeModel, "judge") | ||
|
Comment on lines
+288
to
+292
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On a normal resume the default phase list includes Useful? React with 👍 / 👎. |
||
| } | ||
| this.checkpointManager.save(checkpoint) | ||
|
|
||
| const provider = createProvider(providerName) | ||
| await provider.initialize(getProviderConfig(providerName)) | ||
|
|
||
|
|
@@ -300,8 +331,10 @@ export class Orchestrator { | |
| } | ||
|
|
||
| if (phases.includes("evaluate")) { | ||
| const judge = createJudge(judgeName) | ||
| const judgeConfig = getJudgeConfig(judgeName) | ||
| const judge = cliLlmBackend() ? new CliJudge() : createJudge(judgeName) | ||
| const judgeConfig = cliLlmBackend() | ||
| ? { apiKey: "", model: judgeModel } | ||
| : getJudgeConfig(judgeName) | ||
| judgeConfig.model = judgeModel | ||
| await judge.initialize(judgeConfig) | ||
| await runEvaluatePhase( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,16 @@ import { buildContextString } from "../../types/prompts" | |
| import { ConcurrentExecutor } from "../concurrent" | ||
| import { resolveConcurrency } from "../../types/concurrency" | ||
| import { countTokens } from "../../utils/tokens" | ||
| import { | ||
| cliCallTelemetryFromError, | ||
| cliCallsFromPhase, | ||
| cliComplete, | ||
| cliLlmBackend, | ||
| cliLlmModelId, | ||
| reconcileCliProvenanceIdentity, | ||
| summarizeCliLedger, | ||
| type CliCallTelemetry, | ||
| } from "../../utils/cli-llm" | ||
|
|
||
| type LanguageModel = | ||
| | ReturnType<typeof createOpenAI> | ||
|
|
@@ -46,7 +56,7 @@ function getAnsweringModel(modelAlias: string): { | |
| } | ||
| } | ||
|
|
||
| function buildAnswerPrompt( | ||
| export function buildAnswerPrompt( | ||
| question: string, | ||
| context: unknown[], | ||
| questionDate?: string, | ||
|
|
@@ -79,6 +89,18 @@ export async function runAnswerPhase( | |
| ? questions.filter((q) => questionIds.includes(q.questionId)) | ||
| : questions | ||
|
|
||
| const missingSearchResults = targetQuestions.filter((question) => { | ||
| const checkpointQuestion = checkpoint.questions[question.questionId] | ||
| if (!checkpointQuestion || checkpointQuestion.phases.answer.status === "completed") return false | ||
| const search = checkpointQuestion.phases.search | ||
| return search.status === "completed" && (!search.resultFile || !existsSync(search.resultFile)) | ||
| }) | ||
| if (missingSearchResults.length > 0) { | ||
| throw new Error( | ||
| `Cannot answer because a completed search result file is missing for: ${missingSearchResults.map((question) => question.questionId).join(", ")}` | ||
| ) | ||
| } | ||
|
|
||
| const pendingQuestions = targetQuestions.filter((q) => { | ||
| const status = checkpointManager.getPhaseStatus(checkpoint, q.questionId, "answer") | ||
| const searchStatus = checkpointManager.getPhaseStatus(checkpoint, q.questionId, "search") | ||
|
|
@@ -89,15 +111,19 @@ export async function runAnswerPhase( | |
| }) | ||
|
|
||
| if (pendingQuestions.length === 0) { | ||
| updateAnswerCliLedger(checkpoint, checkpointManager) | ||
| logger.info("No questions pending answering") | ||
| return | ||
| } | ||
|
|
||
| const { client, modelConfig } = getAnsweringModel(checkpoint.answeringModel) | ||
| const useCli = cliLlmBackend() !== null | ||
| const { client, modelConfig } = useCli | ||
| ? { client: null, modelConfig: getModelConfig(DEFAULT_ANSWERING_MODEL) } | ||
| : getAnsweringModel(checkpoint.answeringModel) | ||
| const concurrency = resolveConcurrency("answer", checkpoint.concurrency, provider?.concurrency) | ||
|
|
||
| logger.info( | ||
| `Generating answers for ${pendingQuestions.length} questions using ${modelConfig.displayName} (concurrency: ${concurrency})...` | ||
| `Generating answers for ${pendingQuestions.length} questions using ${useCli ? cliLlmModelId("answerer") : modelConfig.displayName} (concurrency: ${concurrency})...` | ||
| ) | ||
|
|
||
| await ConcurrentExecutor.execute( | ||
|
|
@@ -107,6 +133,10 @@ export async function runAnswerPhase( | |
| "answer", | ||
| async ({ item: question, index, total }) => { | ||
| const resultFile = checkpoint.questions[question.questionId].phases.search.resultFile! | ||
| const priorLlmCalls = cliCallsFromPhase( | ||
| checkpoint.questions[question.questionId].phases.answer | ||
| ) | ||
| let llmCall: CliCallTelemetry | undefined | ||
|
|
||
| const startTime = Date.now() | ||
| checkpointManager.updatePhase(checkpoint, question.questionId, "answer", { | ||
|
|
@@ -129,25 +159,36 @@ export async function runAnswerPhase( | |
| // custom prompt functions that transform context (e.g. Zep's XML-like tags). | ||
| const contextTokens = Math.max(0, promptTokens - basePromptTokens) | ||
|
|
||
| const params: Record<string, unknown> = { | ||
| model: client(modelConfig.id), | ||
| prompt, | ||
| maxTokens: modelConfig.defaultMaxTokens, | ||
| } | ||
|
|
||
| if (modelConfig.supportsTemperature) { | ||
| params.temperature = modelConfig.defaultTemperature | ||
| let text: string | ||
| if (useCli) { | ||
| text = await cliComplete(prompt, { | ||
| role: "answerer", | ||
| retry: false, | ||
|
Comment on lines
+164
to
+166
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| onTelemetry: (telemetry) => { | ||
| llmCall = telemetry | ||
| }, | ||
| }) | ||
| } else { | ||
| const params: Record<string, unknown> = { | ||
| model: client!(modelConfig.id), | ||
| prompt, | ||
| maxTokens: modelConfig.defaultMaxTokens, | ||
| } | ||
| if (modelConfig.supportsTemperature) { | ||
| params.temperature = modelConfig.defaultTemperature | ||
| } | ||
| text = (await generateText(params as Parameters<typeof generateText>[0])).text | ||
| } | ||
|
|
||
| const { text } = await generateText(params as Parameters<typeof generateText>[0]) | ||
|
|
||
| const durationMs = Date.now() - startTime | ||
| checkpointManager.updatePhase(checkpoint, question.questionId, "answer", { | ||
| status: "completed", | ||
| hypothesis: text.trim(), | ||
| promptTokens, | ||
| basePromptTokens, | ||
| contextTokens, | ||
| llmCall, | ||
| llmCalls: llmCall ? [...priorLlmCalls, llmCall] : priorLlmCalls, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Failed retries inflate the per-question llmCalls ledger without distinguishing completed vs failed attempts On the success path the phase records Category: Runtime correctness Why this matters: The CLI ledger/provenance is the disclosure mechanism this PR adds; a 'complete' ledger over a dropped attempt misrepresents which model actually produced (or failed to produce) an answer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If a question has a failed CLI answer attempt and the run is resumed with Useful? React with 👍 / 👎. |
||
| completedAt: new Date().toISOString(), | ||
| durationMs, | ||
| }) | ||
|
|
@@ -159,10 +200,13 @@ export async function runAnswerPhase( | |
| ) | ||
| return { questionId: question.questionId, durationMs } | ||
| } catch (e) { | ||
| llmCall ||= cliCallTelemetryFromError(e) | ||
| const error = e instanceof Error ? e.message : String(e) | ||
| checkpointManager.updatePhase(checkpoint, question.questionId, "answer", { | ||
| status: "failed", | ||
| error, | ||
| llmCall, | ||
| llmCalls: llmCall ? [...priorLlmCalls, llmCall] : priorLlmCalls, | ||
| completedAt: new Date().toISOString(), | ||
| durationMs: Date.now() - startTime, | ||
| }) | ||
|
|
@@ -173,5 +217,26 @@ export async function runAnswerPhase( | |
| } | ||
| ) | ||
|
|
||
| updateAnswerCliLedger(checkpoint, checkpointManager) | ||
|
|
||
| logger.success("Answer phase complete") | ||
| } | ||
|
|
||
| function updateAnswerCliLedger( | ||
| checkpoint: RunCheckpoint, | ||
| checkpointManager: CheckpointManager | ||
| ): void { | ||
| if (!checkpoint.answererProvenance) return | ||
| const phases = Object.values(checkpoint.questions).map((question) => question.phases.answer) | ||
| const ledger = summarizeCliLedger(phases) | ||
| if (ledger.callCount === 0) return | ||
| Object.assign(checkpoint.answererProvenance, { | ||
| callCount: ledger.callCount, | ||
| retryCount: ledger.retryCount, | ||
| executionIdentityCount: ledger.executionIdentityCount, | ||
| mixedExecutionIdentity: ledger.mixedExecutionIdentity, | ||
| callLedgerComplete: ledger.callLedgerComplete, | ||
| }) | ||
| reconcileCliProvenanceIdentity(checkpoint.answererProvenance, ledger.calls) | ||
| checkpointManager.save(checkpoint) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the server creates an advanced copy it always passes
answeringModel || sourceCheckpoint.answeringModeland a requiredjudgeModel, so these truthy checks clear both provenance fields even when the copied run starts fromevaluateorreportand the corresponding completed answer/evaluation phases are kept. That produces reports with retained CLI answers/judgments but no actual execution identity, which regresses the provenance this change is trying to preserve; only clear provenance when that phase is actually reset or the model really changes.Useful? React with 👍 / 👎.