Refactor model download into EnsureModelDownloadedTask#202
Merged
Conversation
The eval tasks split their logic into thin task wrappers around free functions instead of living inside the task, as the libs task idiom intends. EvalS1Task delegated its entire body to runOracleEval, which was imported by nothing else and had no test of its own. - Inline runOracleEval (the oracle comparison over real committed S-1 sections) directly into EvalS1Task.execute, moving its helpers (runSection, summarize) and report types (OracleReport, OracleRunResult, OracleModelSummary, GOLDEN_REFERENCE) into the task file. The task now drives progress through its own execute context rather than an onProgress shim, and prefetches models through the real context. Delete the now-dead runOracleEval.ts; the CLI imports OracleReport from the task. - Own the AI subtasks the extraction path instantiates. runStructured (StructuredGenerationTask) and ensureModelDownloaded (ModelDownloadTask) now wrap the instance in context.own(...), so when a task threads its real execute context down (form processing, the eval prefetch) the subtask is registered in that task's graph and inherits its registry + abort signal. Against the deliberate one-shot stub context (eval sweeps, unit tests), own is an identity no-op, so behavior there is unchanged. The sibling run functions (runExtractionEval, runUnitTermsEval) are left as standalone: they are unit-tested directly and export helpers consumed elsewhere, so inlining them would break tests and duplicate shared code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhM8n9LKToUTHesCuYaXNT
…der from the model id The download seam was a free function handed a pre-resolved ModelConfig. Promote it to a proper task and let it figure the provider out from the model name. - EnsureModelDownloadedTask takes a model **id** and derives the provider (and its download config) from the id shape via secModelRecord — a gguf: id → node-llama-cpp, an org/name id → HuggingFace ONNX, a gpt-*/gemini-*/grok-* id → the matching cloud provider, else Anthropic — instead of requiring a resolved ModelConfig. It owns the ModelDownloadTask (context.own), so the download is registered in the running task's graph and inherits its registry + abort signal, and forwards the download's phase events to the CLI progress UI. Cloud ids no-op; a bare-path GGUF (no model_url) is skipped; the per-id memo still pays a download at most once per process. - ensureModelDownloaded / prefetchModel become thin wrappers that own + run the task on the caller's context (prefetchModel best-effort, swallowing failures). Both now take a model id string rather than a ModelConfig. - Update callers to pass the id they already have: the eval loops and EvalS1Task pass the candidate id (dropping a redundant repository resolve in the prefetch loop), the form processors pass their computed model_id, and runStructured's safety-net passes resolveModelId(model). Tests rewritten to drive the task by id and to cover the provider-from-id dispatch, the owned-download progress forwarding across the two task layers, memoization, and a standalone EnsureModelDownloadedTask.run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhM8n9LKToUTHesCuYaXNT
…and name the file after the task The download task was the one task file not named after its task and not living under src/task/. Rename src/config/ensureModelDownloaded.ts to src/task/model/EnsureModelDownloadedTask.ts (and its test alongside), matching the convention every other task file already follows, and move it out of the DI-config folder into a new model/ task domain. Update the internal secModelRecord import and all nine importers to the new path; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhM8n9LKToUTHesCuYaXNT
The SEC fetch tasks (SecCachedFetchTask, SecFetchTask, SecFetchJob, SecJobQueue, SecFetchFileOutputCache) are tasks/task infrastructure, so they belong under src/task/ with the rest. Move the whole src/fetch directory to src/task/fetch, rewrite the moved files' outward imports (../config|util|cli → ../../…) and every importer's path (task-domain siblings ../../fetch → ../fetch; commands/index.ts ../fetch → ../task/fetch). Pure relocation, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhM8n9LKToUTHesCuYaXNT
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves the model download harness from a utility function (
ensureModelDownloadedinsrc/config/ensureModelDownloaded.ts) into a proper Workglow task (EnsureModelDownloadedTaskinsrc/task/model/EnsureModelDownloadedTask.ts), and inlines the oracle evaluation logic directly intoEvalS1Task.Summary
This refactor improves task graph visibility and progress reporting for model downloads by making them first-class tasks that integrate with the CLI's task-graph UI. It also consolidates the oracle evaluation implementation into the task that runs it, eliminating a separate
runOracleEvalfunction.Key Changes
New task:
EnsureModelDownloadedTaskwrapsModelDownloadTaskand derives the provider (and download config) from the model id shape alone viasecModelRecord, eliminating the need to pass a resolvedModelConfig. The task:model_url(assumed on disk)ModelDownloadTaskfor HuggingFace ONNX and remote GGUF idscontext.updateProgressso multi-GB fetches render live in the CLI UIInlined oracle logic:
EvalS1Task.execute()now contains the full oracle evaluation implementation (previously inrunOracleEval), including:context.updateProgressRemoved files:
src/eval/runOracleEval.ts(logic moved toEvalS1Task)src/config/ensureModelDownloaded.ts(replaced by task)src/config/ensureModelDownloaded.test.ts(tests moved to task)Updated imports: All callers of
ensureModelDownloaded/prefetchModelnow import fromsrc/task/model/EnsureModelDownloadedTaskFile reorganization: Fetch-related tasks moved from
src/fetch/tosrc/task/fetch/for consistency with the task-based architectureImplementation Details
EnsureModelDownloadedTaskusescontext.own()to register the download as a subtask, ensuring it inherits the running task's registry, abort signal, and progress UIhttps://claude.ai/code/session_01BhM8n9LKToUTHesCuYaXNT