Support Ollama Cloud and make interrupted runs resumable - #1
Open
comsa33 wants to merge 20 commits into
Open
Conversation
Prepare for open-weight model experiments against a rate-limited provider. Resumption previously keyed on list position and counted every checkpoint line as done, so an item that failed was frozen into the results as a permanent gap and its question was never retried. Against a provider that rate-limits, that silently shrinks n. Resumption now keys on question id and treats only successful items as done, so a rerun spends calls solely on the outstanding questions. - resume by question id; failed items return to the work list - write the checkpoint after every item, atomically via a temp file, so an interrupt costs at most the item in flight instead of up to ten - retry an item in place with exponential backoff before recording failure - route "ollama_cloud/<model>" through the OpenAI-compatible endpoint - allow RESULTS_DIR to be overridden so a run cannot overwrite existing results, and DISABLE_LLM_CACHE for valid latency measurement
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughOllama Cloud 및 추론 모델 설정을 확장하고, 실험 파이프라인에 캐시 비활성화, ID 기반 재개, 아이템별 재시도와 원자적 체크포인트를 적용했습니다. 결과 통계는 질문 ID 기준으로 페어링하며, variant 선택과 평가 산출을 지원합니다. Changes실험 실행 및 모델 구성
결과 통계 페어링
Variant 선택 및 평가 실행
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ExperimentSetup
participant DSPy
participant DatasetRunner
participant Pipeline
participant Checkpoint
ExperimentSetup->>DSPy: 캐시 비활성화 및 LM 구성
DatasetRunner->>Checkpoint: 성공 ID 로드
DatasetRunner->>Pipeline: 미완료 항목 실행
Pipeline-->>DatasetRunner: 성공 결과 또는 예외
DatasetRunner->>Pipeline: 실패 항목 재시도
DatasetRunner->>Checkpoint: 아이템별 원자적 저장
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
agentic_rag/config/settings.py (2)
196-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOllama Cloud 라우팅에 대한 단위 테스트 부재.
make_lm의 새 분기(모델명 재작성, api_base/api_key 주입)를 검증하는 테스트가 제공된 파일 목록에 없습니다. 이 PR은 재개/체크포인트 로직에 5개 테스트를 추가했지만, 라우팅 로직 회귀를 잡을 테스트는 없어 보입니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agentic_rag/config/settings.py` around lines 196 - 203, Add unit tests for the Ollama Cloud branch in make_lm, covering ollama_cloud/<model> rewriting to openai/<model> and injecting the configured api_base and api_key defaults. Also verify non-Ollama Cloud models retain their existing routing behavior.
196-203: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
OLLAMA_API_KEY미설정 시 조기 검증 부재.
settings.ollama_api_key기본값이 빈 문자열이라, 환경변수를 설정하지 않고ollama_cloud/<model>을 사용하면dspy.LM(model, api_key="", ...)로 빈 키가 그대로 전달됩니다. 요청 시점에야 불명확한 인증 오류가 발생하므로, 레이트 리밋된 클라우드 실험 도중 디버깅 비용이 커질 수 있습니다.🛠️ 제안: 조기에 명확한 오류 발생
if model.startswith(OLLAMA_CLOUD_PREFIX): + if not settings.ollama_api_key: + raise ValueError( + "OLLAMA_API_KEY is required when using an ollama_cloud/ model." + ) defaults.setdefault("api_base", settings.ollama_api_base) defaults.setdefault("api_key", settings.ollama_api_key) model = f"openai/{model[len(OLLAMA_CLOUD_PREFIX) :]}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agentic_rag/config/settings.py` around lines 196 - 203, Validate that settings.ollama_api_key is non-empty before configuring an OLLAMA_CLOUD_PREFIX model in the Ollama Cloud handling block. Raise a clear configuration error immediately when the key is missing, and preserve the existing api_base, api_key, and model transformation for configured keys.experiments/common.py (1)
141-169: 🗄️ Data Integrity & Integration | 🔵 Trivial체크포인트 파일은 동시 다중 프로세스 실행을 가정하지 않음.
_save_checkpoint가 매번results전체를 다시 써서 원자적으로 교체하는 방식이라, 동일한checkpoint_dir/pipeline_name으로 두 프로세스를 동시에 실행하면 나중에 저장하는 쪽이 다른 프로세스의 진행상황을 덮어쓸 수 있습니다. 레이트 리밋 완화를 위해 향후 병렬 워커로 확장할 계획이 있다면 이 부분을 파일 락 또는 프로세스별 체크포인트 파일로 보완할 필요가 있습니다. 현재는 순차 단일 프로세스 설계로 보이므로 당장 문제는 아닙니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@experiments/common.py` around lines 141 - 169, The current checkpoint implementation assumes a single sequential process; no immediate code change is required. If concurrent workers are introduced, update _save_checkpoint and checkpoint loading to coordinate shared writes with a file lock or use process-specific checkpoint files, while preserving atomic checkpoint replacement and merged progress.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@agentic_rag/config/settings.py`:
- Around line 196-203: Add unit tests for the Ollama Cloud branch in make_lm,
covering ollama_cloud/<model> rewriting to openai/<model> and injecting the
configured api_base and api_key defaults. Also verify non-Ollama Cloud models
retain their existing routing behavior.
- Around line 196-203: Validate that settings.ollama_api_key is non-empty before
configuring an OLLAMA_CLOUD_PREFIX model in the Ollama Cloud handling block.
Raise a clear configuration error immediately when the key is missing, and
preserve the existing api_base, api_key, and model transformation for configured
keys.
In `@experiments/common.py`:
- Around line 141-169: The current checkpoint implementation assumes a single
sequential process; no immediate code change is required. If concurrent workers
are introduced, update _save_checkpoint and checkpoint loading to coordinate
shared writes with a file lock or use process-specific checkpoint files, while
preserving atomic checkpoint replacement and merged progress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c7fc9cf6-f145-498a-a9ac-e0e54bfa1236
📒 Files selected for processing (3)
agentic_rag/config/settings.pyexperiments/common.pytests/test_experiment_resume.py
Paired tests aligned the two score arrays by list position and truncated to the shorter one. Results are loaded with failed items filtered out, so a single dropped item shifted every later position and the test compared answers to different questions — while still reporting itself as paired. Existing published runs have no failures, so their numbers are unaffected (verified: 2Wiki TARA vs Loop +0.0897, vs CRAG +0.3289, matching Table 3). Resumable runs against a rate-limited provider will produce gaps and out-of-order retries, which is what makes this reachable. Pair on question id, take the intersection, and warn when a comparison covers fewer questions than either run holds.
Three defects surfaced while validating gpt-oss against the pipeline. max_tokens was defined in settings but never handed to the LM, so the provider default applied. Providers with a small default truncate the response mid-structure, which surfaces as unparsable tool calls rather than as an error — gpt-oss produced null tool names and a decompose crash, while Gemini and gpt-5-mini were unaffected because their defaults are generous. Passing it then broke gpt-5-mini: DSPy rejects the gpt-5 family unless max_tokens is at least 16000. Reasoning models now get that floor, which also gives gpt-oss headroom since reasoning tokens share the budget. Reasoning effort is now a setting rather than a hardcoded "low". Holding gpt-oss at "low" starves it of the reasoning it needs to complete a JSON schema, producing responses cut off after a few characters. The default stays "low" so gpt-5-mini reproduces exactly; "default" omits the parameter for models that need their own budget.
Pipelines differ by an order of magnitude in LLM calls per question — on gpt-oss:120b, CRAG takes 122s per question against Naive's 16s and makes 52 calls against 1. Run sequentially, a five-pipeline experiment costs the sum of all of them, and CRAG alone is roughly half the wall clock. --variants already existed for --ablation; extend it to --config so the expensive pipelines can run as separate concurrent processes. Checkpoints are keyed per variant, so splitting a run across processes is safe.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agentic_rag/config/settings.py (1)
215-225: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win추론 모델의
max_tokens하한은kwargsmerge 이후에도 유지하세요.
defaults.update(kwargs)가REASONING_MIN_MAX_TOKENS로 강제한 16,000 하한을 caller의 작은max_tokens로 덮을 수 있습니다. 예를 들어 GPT-5 계열에max_tokens=4096을 전달하면 호출 시점이 거부되거나 응답이 잘릴 수 있으니,kwargsmerge 후에도defaults["max_tokens"] = max(defaults["max_tokens"], REASONING_MIN_MAX_TOKENS)로 적용하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agentic_rag/config/settings.py` around lines 215 - 225, After defaults.update(kwargs) in the model configuration flow, reapply the REASONING_MIN_MAX_TOKENS lower bound to defaults["max_tokens"]. Ensure caller-provided values below the minimum are raised to the minimum while preserving larger values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agentic_rag/config/settings.py`:
- Around line 199-203: Update make_lm() to include settings.disable_llm_cache in
the shared LM configuration, so direct dspy.context(lm=make_lm(...)) calls honor
the cache-disabled setting just like setup_experiment(). Pass the value through
the existing DSPy LM/cache configuration mechanism while preserving the current
temperature, retry, and token defaults.
In `@experiments/analysis/significance.py`:
- Around line 57-63: Update the pairing logic around the valid-results
enumeration to preserve each record’s original position from the unfiltered
items when generating fallback IDs; do not use the post-filter `position` for
ID-less legacy results. Alternatively, reject comparisons when such files
contain failures, and add a regression test covering mismatched failure
positions across pipelines so different questions cannot be paired.
In `@experiments/run.py`:
- Line 523: Update the run_ablation variant selection logic to validate every
requested name against the configured variants and reject unknown names instead
of silently filtering them out. Preserve the existing --config validation
behavior and ensure an all-invalid request cannot proceed with zero variants.
---
Outside diff comments:
In `@agentic_rag/config/settings.py`:
- Around line 215-225: After defaults.update(kwargs) in the model configuration
flow, reapply the REASONING_MIN_MAX_TOKENS lower bound to
defaults["max_tokens"]. Ensure caller-provided values below the minimum are
raised to the minimum while preserving larger values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6d2f32c-606d-46f1-8f08-cf4f3a12dcb9
📒 Files selected for processing (4)
agentic_rag/config/settings.pyexperiments/analysis/significance.pyexperiments/run.pytests/test_significance_pairing.py
Two defects from the CodeRabbit review on PR #1. The positional fallback in significance analysis numbered the surviving records rather than the records in the file. A failure renumbered everything after it, so for result files predating the id field the fallback misaligned questions in precisely the case it exists to cover. Positions now come from the original file. run_ablation still dropped unknown variant names silently while the help text claimed both paths behave alike. A typo ran a subset of the requested variants, or none at all, and still exited successfully — the worst failure mode for an experiment runner, since the gap surfaces only when results are assembled. It now rejects unknown names like --config does. Adds regression tests for the legacy-file fallback and for variant rejection on both execution paths.
Wraps the four-dataset RQ1 sweep on Ollama Cloud, split across three workers so the pipelines run concurrently within the provider's 3-request limit. Datasets are listed in order and checkpoints are keyed on question id, so rerunning after an interrupt costs no calls for work already done — the runner reads each checkpoint and moves on. Pins the settings gpt-oss needs: reasoning effort left at the provider default (holding it at "low" truncates structured responses) and the LLM cache disabled so latency remains measurable.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/run_openweight.sh`:
- Line 25: Preserve caller-provided RESULTS_DIR by changing its default
assignment to use the existing environment value when set, falling back to
data/results-openweight otherwise. Update the related LOG_DIR assignment to
derive from RESULTS_DIR/logs so results, checkpoints, and logs share the
configured base directory.
- Around line 49-59: Accumulate failures from each `uv run python
experiments/run.py` invocation in the dataset loop, while continuing to process
subsequent datasets. Update the final script exit status after the loop so any
failed dataset causes a non-zero exit instead of reporting the sweep as
successful; preserve the per-dataset completion logging and `ALL DATASETS
COMPLETE` message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fddcd87e-228f-4f6d-a7dc-3c96720a965a
📒 Files selected for processing (5)
experiments/analysis/significance.pyexperiments/run.pyscripts/run_openweight.shtests/test_run_variants.pytests/test_significance_pairing.py
🚧 Files skipped from review as they are similar to previous changes (2)
- experiments/run.py
- experiments/analysis/significance.py
Two review findings on the open-weight sweep runner. A failing dataset was logged and then ignored: the loop moved on, printed ALL DATASETS COMPLETE, and exited 0. A supervising process could not tell a finished sweep from one that lost a dataset — the same failure mode as silently dropping an unknown variant, which this PR already fixed elsewhere. Failures now accumulate and the script exits non-zero naming the datasets that failed. RESULTS_DIR was overwritten unconditionally, so a caller-supplied results directory was ignored while LOG_DIR still pointed at the hardcoded default, splitting results and logs across trees. All tuning variables now default without clobbering, and LOG_DIR follows RESULTS_DIR. Verified: injecting a failing command yields exit=1 with the failed datasets named, and RESULTS_DIR=/tmp/x places logs under /tmp/x/logs.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/run_openweight.sh (1)
59-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
spec파싱에서 비인용 확장을 제거하세요.
set -- $spec는 word splitting과 pathname expansion을 수행합니다. 현재 고정된 값에서는 문제가 없지만, dataset spec이 추가되거나 특수문자를 포함하면ds와n이 잘못 파싱될 수 있습니다.read -r로 문자열을 안전하게 분해하세요.수정 예시
- set -- $spec - ds="$1"; n="$2" + read -r ds n <<< "$spec"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/run_openweight.sh` around lines 59 - 60, Replace the unquoted `set -- $spec` parsing in the spec-processing flow with `read -r`-based splitting so word splitting and pathname expansion cannot alter dataset values; continue assigning the parsed fields to `ds` and `n` with the existing behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/run_openweight.sh`:
- Around line 59-60: Replace the unquoted `set -- $spec` parsing in the
spec-processing flow with `read -r`-based splitting so word splitting and
pathname expansion cannot alter dataset values; continue assigning the parsed
fields to `ds` and `n` with the existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: beb103e2-35b7-443b-a1c3-61224742d405
📒 Files selected for processing (1)
scripts/run_openweight.sh
Only data/results/ was ignored, so the trees added for the open-weight comparison (results-openweight, results-glm, results-latency) and the human annotation sheets were staged as 172 untracked files — 26MB into a public repository, including the raw annotator labels. Covers them with data/results-*/, and adds .omc/ alongside the existing .claude/ entry since both are agent state rather than source.
Multi-judge robustness re-scores the same results with several judges, but the output path was a fixed _judged.jsonl, so each judge silently overwrote the previous one's verdicts. Tag both the per-file output and the summary with the judge model; omitting --judge-model keeps the old names so existing files stay addressable. make_human_eval_csv.py builds the labelling sheet behind the judge/human kappa. It carries no judge column and shuffles rows, so the labeller can infer neither the verdict nor the pipeline; sampling is stratified across pipelines so agreement is measured over the full quality range rather than only on easy cases.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/make_human_eval_csv.py`:
- Around line 55-63: Remove the pipeline field from the rows produced by the
CSV-building logic, including the visible output generated around rows.append.
Keep only an opaque sample identifier and the fields required for blind
labeling, while storing the (dataset, pipeline, id) mapping separately in a
private output so pipeline names are not exposed to labelers.
In `@scripts/run_llm_judge.py`:
- Around line 44-45: Update the artifact-key construction around judge_model and
the tag value to preserve the full model identity, safely encode it, and append
a deterministic short hash of the complete judge_model. Ensure distinct model
paths cannot produce the same judge artifact or summary key while keeping the
result concise and stable across runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 33767b88-010e-4497-88fa-41a6d0e47e8c
📒 Files selected for processing (3)
.gitignorescripts/make_human_eval_csv.pyscripts/run_llm_judge.py
Reasoning models draw the answer and the hidden reasoning from one token budget, so they need the 16000 floor rather than the ordinary 4096. The check listed only gpt-5 and gpt-oss, and a model outside that list fails silently: it truncates mid-answer while the run still reports success. glm-5.2 ran a full dataset that way. 0.84% of its calls were truncated, but unevenly by pipeline — 4.18% of Naive calls against 0.44% of Agentic ones, because Naive spends its single call on the answer itself while Agentic spreads eleven calls across tools. That skew flatters the proposed method, so those results are not usable. Because a model is selected through environment variables, requiring a code edit here is the trap itself; LLM_REASONING_MODELS now overrides the list. Models already measured keep their conditions: Gemini and gpt-4.1-nano stay at 4096, gpt-oss stays at 16000.
…uely Two findings from the review of 2ff61de. The labelling sheet shuffled its rows so a labeller could not infer the pipeline from their order, then printed the pipeline as a column. Anyone filling it in could read off agentic_(react) — the proposed method — against naive_rag. The sheet now carries an opaque sample id, and the mapping back to (dataset, pipeline, id) goes to a separate key file that the labellers must not receive. The judge suffix dropped the provider prefix and flattened ':' and '.', so openai/qwen3-5 and ollama_cloud/qwen3.5 produced the same filename. Two judges sharing one is worse than an ugly name: the second run resumes from the first one's checkpoint, skips every item it believes is done and overwrites the summary. A digest of the full model string now disambiguates. Verdicts already written under the digest-less name keep it — those cost API calls, and the summaries never recorded the model string, so the right digest for an existing file cannot be recomputed.
The LLM judge is one of the paper's three headline metrics, so the kappa behind it has to come from a module that reruns on the raw sheets rather than from a spreadsheet nobody can re-derive. Table 12's refusal rates already cost this project a day for exactly that reason. Intervals are bootstrapped rather than taken from the asymptotic standard error, because the per-dataset cells are n=50 and several are near-degenerate, where the normal approximation for kappa is not trustworthy. Judge agreement is scored only on items the two annotators settled between them, so the judge is never marked wrong on a case the humans could not agree on either; the dropped count is reported alongside.
deepseek-v4-flash truncated 2 of its first 14 verdicts at max_tokens=4096 while judging, the same failure that cost glm-5.2 a full dataset. A three-item smoke probe had passed it cleanly — at a rate near one in seven the probe simply had no power to see it, which is why the runner counts truncation warnings across the whole sweep and says so in its log rather than trusting the exit code. The list now errs toward including a model that turns out not to need the larger ceiling: max_tokens caps rather than targets, so a needless entry changes nothing, while a missing one silently corrupts a run. run_multi_judge.sh re-scores every RQ1 directory with one judge per process. Judging is checkpointed per item, so the sweep resumes after Ollama's session quota cuts it off, and each judge's output carries its own tag so three concurrent runs cannot collide.
Answers the question the Judge column invites — whether it is an artifact of gpt-4.1-nano — by scoring the same human-labelled panel with four more judges and reporting agreement with the annotators, agreement between judges, and whether the pipeline ranking moves. Panel items join on (pipeline, id) rather than id: the same question appears under several pipelines, so 200 rows carry only 184 distinct ids and joining on id alone would pair one pipeline's verdict with another's answer without any error surfacing. As in kappa_analysis, judges are scored only where both annotators agreed, so none is marked wrong on a case the humans split on.
A judge whose completion hits max_tokens returns no verdict: the boolean field parses to None, which is falsy, so 'is_correct else 0.0' recorded it as the judge calling the answer wrong. Forcing a truncation on six items shows what that costs — judge accuracy read 0.333, and 0.667 once the same items were scored with room to finish. The bias is not random. Longer, more complex answers draw more hidden reasoning and truncate more often, so the silent zeros land unevenly across pipelines, which is the same shape of error that made the glm-5.2 run unusable. Verdicts now carry judge_status. Anything but 'ok' is dropped from the checkpoint on the next run, so rerunning with a larger ceiling re-scores only those items and leaves the rest untouched; the file is rewritten first so a replacement never sits alongside what it replaced. A finished run can now prove it is clean rather than being assumed so — the earlier sweep had no way to say which four of its four thousand verdicts came from a cut-off response.
A results file can hold the same question once per pipeline. The judge panel is exactly that shape — 200 rows over 184 ids — so the checkpoint lookup collapsed 16 rows, and the rewrite that drops unusable verdicts then wrote the file back without them. The panel lost 16 verdicts on its first resume before this was caught. Rewriting now works from the row list rather than the lookup, so rows that share an id survive, and tests cover both halves: that two pipelines' verdicts for one question stay distinct, and that an unusable verdict is absent from the checkpoint so a rerun re-scores it. The hint printed after an incomplete run no longer tells the reader to raise LLM_MAX_TOKENS unconditionally — an unparsed verdict finished within its budget and more room will not change it. judge_panel skips a panel file scored by the baseline judge, which is kept as a reproducibility check but was being reported as a second judge.
…udge Scores every RQ1 result again with an alternative judge and compares what the paper actually claims — TARA over the fixed loop — rather than which pipeline happens to top a dataset. That first framing was tried and discarded: it reported four disagreements out of twelve, and every one was a tie or a sub-point difference, including a dataset where the two candidates both landed on 87.0% and max picked between them. Verdicts whose judge_status is not 'ok' are excluded rather than scored as 0.0, since counting a completion that produced no judgement as 'wrong' penalises whichever pipelines write the longest answers.
Scores the 200 items behind the human labels with one judge after another, so every judge is compared against the same annotators on the same answers. That is what answers whether the reported Judge column depends on its judge, and at a fiftieth of the cost of re-scoring all 11,250 results. Judges are arguments rather than a fixed list so the caller controls concurrency: Ollama Cloud allows three requests at once and the full re-scoring sweep holds one of them. Truncation warnings are counted per judge and reported, because a three-item probe passed deepseek-v4-flash and it then truncated 2 of its first 14 verdicts.
A saved summary carried top_k, hybrid_weight, quality_threshold, max_retry and seed — and not the fields that actually distinguished one run from another. max_passages is the one that just cost a day: only the accumulating pipelines apply it, so in every published run the baselines carried 50 passages against the proposed method's 30, and nothing on disk said so. It took reading passages_used across twelve result sets to find, four months after the fact. enabled_tools is why Table 3 and Table 11 disagree — RQ1 ran four tools where RQ3/RQ4 ran all of them. max_tokens is what silently truncated a reasoning model's answers for an entire dataset. Model names were recoverable only from the directory name. Tests assert the summary carries them and follows the values in force rather than hardcoding, since a sweep varies them per run.
The previous commit added max_passages and the rest to the summary, and a smoke run showed it recording top_k=50 for all six variants across two conditions. Variants run one after another and every result is saved after the loop finishes, so reading the global settings at save time captured whichever variant went last. The fix reproduced the very failure it was written to prevent — a file that cannot say which condition produced it. _run_variant now snapshots while its own settings are in force and hands that to save_results. Verified by rerunning the smoke: the (30,30) and (50,30) variants now record their own values, and passages_used matches. Adds the FinanceBench retrieval-space sweep config. It moves top_k and max_passages together, because max_passages caps the final context at 30 — so any top_k above it gives the accumulating pipelines an identical run — while Naive RAG never applies that cap, so moving top_k alone would change only Naive's condition and read as 'more passages favour Naive'. Its (30,30) point is also the controlled comparison the published runs lacked.
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
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.
Why
Preparing open-weight model experiments (gpt-oss:20b / 120b via Ollama Cloud) to close a reproducibility gap: the current primary model is
gemini-3.1-flash-lite-preview, and a preview model that gets deprecated makes the published results impossible to reproduce.Those runs will hit provider rate limits, which exposed a defect in how runs resume.
The resume defect
run_pipeline_on_datasetresumed by list position (start_idx = len(results)) and counted every checkpoint line as done — including lines recording a failure. A rate-limited item was therefore frozen into the results as a permanent gap and its question was never retried, silently shrinking n. Checkpoints were also written only every 10 items, so an interrupt could cost up to nine questions of work (~450 LLM calls for the CRAG pipeline, which issues one call per passage).Changes
ollama_cloud/<model>routes through the OpenAI-compatible endpoint; local Ollama and existing providers are untouchedRESULTS_DIRis now overridable so a new run cannot overwrite existing resultsDISABLE_LLM_CACHEturns the DSPy response cache off. Attemperature=0a rerun replays cached completions in milliseconds — accuracy is unaffected, but measured latency becomes meaninglessVerification
tests/test_experiment_resume.py: completed items are never re-answered, failed items return to the work list, transient failures recover in place, the checkpoint is written per item with no temp file left behind, and progress is counted by successes rather than line countruff checkandruff formatcleantool_calls, which the ReAct pipeline requires to function at allNot included
No experiment has been run yet. Merging is deferred until a pilot shows whether these results belong in the paper.
Summary by CodeRabbit
reasoning_effort를 적용합니다.variant_names기반 변형 선택 필터링 및 CLI/실행 경로 일관성을 추가했습니다.