Ft/syn agent bench - #43
Conversation
📝 WalkthroughWalkthroughAdded the SynBench benchmark system for domain loading, synthetic task generation, LLM-based agent execution, verification, scoring, metrics, notebook workflows, and mock-retail fixtures. Updated project dependencies, lint settings, tests, and review configuration. ChangesSynBench benchmark system
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (15)
implementations/agent_benchmark_generation/domains/mock_retail/policy.md-28-31 (1)
28-31: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winThe refusal wording uses a different spelling from the stored order status.
This section instructs the agent to say
canceled. The domain sets the order status tocancelled, as shown in notebook 2 output line 221. Agents commonly mirror the wording they observe in tool results, so this mismatch degrades theCOMMUNICATEscoring signal. The same mismatch appears inimplementations/agent_benchmark_generation/data/benchmarks/mock_retail/tasks.json.🤖 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 `@implementations/agent_benchmark_generation/domains/mock_retail/policy.md` around lines 28 - 31, Update the cancellation-refusal wording in the Communication section to use “cancelled,” matching the stored order status and the corresponding task benchmark wording. Keep the instruction concise and preserve the requirement to mention that the order cannot be cancelled due to its status.implementations/agent_benchmark_generation/3-single_agent_evaluation.ipynb-210-212 (1)
210-212: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the model name and the tool index label.
Two documentation defects exist in this notebook:
- Line 210 states the default model is
gemini-2.0-flash. The stored output at line 225 showsgemini-3.1-flash-lite-preview. Notebook 4 line 234 repeats the same stale value.- Line 258 prints "first tool", but line 261 indexes
[3], which iscancel_order. Notebook 4 line 260 labels the same index correctly ascancel_order.✏️ Proposed fix
- "print(\"\\nOpenAI-style tool schema (first tool):\")\n", + "print(\"\\nOpenAI-style tool schema (cancel_order):\")\n",Also applies to: 258-261
🤖 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 `@implementations/agent_benchmark_generation/3-single_agent_evaluation.ipynb` around lines 210 - 212, Update the notebook documentation to state that get_client() defaults to gemini-3.1-flash-lite-preview, including the corresponding stale value in notebook 4. Correct the output label near the tool inspection to identify index [3] as cancel_order instead of “first tool,” matching the actual indexed tool and notebook 4.implementations/agent_benchmark_generation/2-generate_and_verify_tasks.ipynb-493-497 (1)
493-497: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not hardcode the draft count in the summary message.
The message prints
/ 3whiledraftscomes fromrun.run(n=3). If a reader changesn, the printed denominator becomes wrong.♻️ Proposed fix
- "print(f\" ({len(verified_tasks)} / 3) passed the verification pipeline.\")\n", + "print(f\" ({len(verified_tasks)} / {len(drafts)}) passed the verification pipeline.\")\n",🤖 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 `@implementations/agent_benchmark_generation/2-generate_and_verify_tasks.ipynb` around lines 493 - 497, Update the verification summary near run.verify_drafts to derive the denominator from the requested draft count or the drafts collection instead of hardcoding 3, so the message remains accurate when run.run(n=...) changes.implementations/agent_benchmark_generation/2-generate_and_verify_tasks.ipynb-531-532 (1)
531-532: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the grammar in the Step 6 markdown.
"It might worth running" is not grammatical.
✏️ Proposed fix
- "Before running evaluation, we need to load the generated tasks. It might worth running verification again in case we are not sure if it was done before." + "Before running evaluation, we need to load the generated tasks. It is worth running verification again if you are not sure it was done before."🤖 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 `@implementations/agent_benchmark_generation/2-generate_and_verify_tasks.ipynb` around lines 531 - 532, Update the Step 6 markdown text to correct the grammar in the phrase “It might worth running,” while preserving the existing meaning and surrounding instructions.implementations/agent_benchmark_generation/README.md-3-5 (1)
3-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the grammar in the intro and prerequisites.
Line 3 has subject-verb disagreement. Line 5 needs a hyphen and is missing a noun after "synbench specific".
✏️ Proposed fix
-These notebooks walks through **SynBench** end to end: loading a domain, generating synthetic benchmark tasks, verifying them, and evaluating tool-calling agents. +These notebooks walk through **SynBench** end to end: loading a domain, generating synthetic benchmark tasks, verifying them, and evaluating tool-calling agents. -**Prerequisites:** from the repo root, run `uv sync --dev --group synbench` to install synbench specific as well as dev dependencies, and start the first Jupyter notebook. Select the kernel and run the cells. +**Prerequisites:** from the repo root, run `uv sync --dev --group synbench` to install the synbench-specific dependencies as well as the dev dependencies. Then start the first Jupyter notebook, select the kernel, and run the cells.🤖 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 `@implementations/agent_benchmark_generation/README.md` around lines 3 - 5, Correct the introductory sentence in the README so “These notebooks” uses the plural verb “walk.” In the prerequisites sentence, hyphenate “SynBench-specific” and add the missing noun “dependencies” after it.Source: Linters/SAST tools
aieng-synthetic-data/aieng/syn_data/synbench/display.py-94-97 (1)
94-97: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard
contentagainstNone.
show_taskformatsinitial_messagewithor '—', which indicates the scenario text can beNoneor empty.show_messagecallscontent.replace(...)without a guard, so aNonevalue raisesAttributeErrorin the notebook. Notebook 3 passestask.user_scenario.initial_messagedirectly toshow_message.🛡️ Proposed guard
-def show_message(content: str, *, role: str = "user", title: str | None = None) -> None: +def show_message( + content: str | None, *, role: str = "user", title: str | None = None +) -> None: """Render a single chat message.""" heading = title or f"{role.capitalize()} message" - _emit(f"### {heading}\n\n> {content.replace(chr(10), chr(10) + '> ')}") + text = content or "*(empty)*" + _emit(f"### {heading}\n\n> {text.replace(chr(10), chr(10) + '> ')}")🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/display.py` around lines 94 - 97, Update show_message to handle a None content value before calling replace, rendering the same placeholder used by show_task for missing scenario text while preserving normal formatting for non-empty content.implementations/agent_benchmark_generation/4-multi_agent_pipeline_evaluation.ipynb-633-637 (1)
633-637: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the step numbering.
The notebook goes from "Step 4" to "Step 6". No "Step 5" exists.
✏️ Proposed fix
- "## Step 6 — Batch metrics (pass@1)\n", + "## Step 5 — Batch metrics (pass@1)\n",🤖 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 `@implementations/agent_benchmark_generation/4-multi_agent_pipeline_evaluation.ipynb` around lines 633 - 637, Update the Step 6 heading in the notebook’s batch metrics section to use the correct sequential step number, Step 5, preserving the existing section title and content.implementations/agent_benchmark_generation/README.md-9-13 (1)
9-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the
.env.examplepath in the README.
.env.exampleis committed at the repository root. Fromimplementations/agent_benchmark_generation/, usecp ../../.env.example .env.🤖 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 `@implementations/agent_benchmark_generation/README.md` around lines 9 - 13, Update the setup command in the README to copy the repository-root .env.example using the correct ../../.env.example relative path from implementations/agent_benchmark_generation/, while preserving the existing instruction to configure OPENAI_API_KEY.Source: Linters/SAST tools
aieng-synthetic-data/aieng/syn_data/synbench/domain/loader.py-95-97 (1)
95-97: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle a top-level seed-task list before calling
.get().The fallback implies that
tasks.seed.jsoncan contain a top-level list. That form fails becauseseed_raw.get(...)runs before the type check. Branch onisinstance(seed_raw, list)first.Proposed fix
- seed_tasks = [Task.model_validate(t) for t in seed_raw.get("tasks", seed_raw)] + raw_tasks = seed_raw if isinstance(seed_raw, list) else seed_raw.get("tasks") + if not isinstance(raw_tasks, list): + raise DomainLoadError("tasks.seed.json must contain a task list") + seed_tasks = [Task.model_validate(task) for task in raw_tasks]🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/domain/loader.py` around lines 95 - 97, Update the seed loading logic in the task loader to check whether seed_raw is a list before calling .get(); use the list directly for top-level list files, otherwise read the tasks key with the existing fallback for mapping-shaped data, then validate each entry with Task.model_validate.aieng-synthetic-data/aieng/syn_data/synbench/fsm/validator.py-38-38 (1)
38-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject actions that reference undeclared tools.
Line 38 maps every unknown tool name to
"lookup". An undeclared action can therefore pass FSM validation when the expected step is a lookup. RaiseFSMValidationErrorbefore constructingaction_tags. Add a regression test for an unknown tool in a lookup path.Proposed fix
allow_write: bool = cfg.get("allow_write", True) tags = _tool_tags(domain) - action_tags = [tags.get(a.name, "lookup") for a in actions] + unknown_tools = [action.name for action in actions if action.name not in tags] + if unknown_tools: + raise FSMValidationError(f"Unknown tool: {unknown_tools[0]}") + + action_tags = [tags[action.name] for action in actions]🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/fsm/validator.py` at line 38, Validate every action’s tool name against the declared tool tags before constructing action_tags in the FSM validator, and raise FSMValidationError for any undeclared tool instead of defaulting it to "lookup". Add a regression test covering an unknown tool used in a lookup path.aieng-synthetic-data/aieng/syn_data/synbench/agents/loop.py-61-69 (1)
61-69: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe duplicate check scans the whole transcript and can drop a legitimate repeated utterance.
The
any(...)scan comparesuser_msgagainst everyusermessage in the session. The stated intent is only to avoid re-seeding the first message. If a customer repeats an identical utterance later in a dialogue (for exampleAny update?), this drops the new turn. The LLM is then called with a transcript that does not contain the newest customer message.Compare against the last
usermessage only.🐛 Proposed fix
- if user_msg and not any( - m.get("role") == "user" and m.get("content") == user_msg - for m in session.messages - ): + last_user = next( + ( + m.get("content") + for m in reversed(session.messages) + if m.get("role") == "user" + ), + None, + ) + if user_msg and last_user != user_msg: session.messages.append({"role": "user", "content": user_msg})🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/loop.py` around lines 61 - 69, Update the initial user-message seeding logic in the session setup around initial_user_message and session.messages so it checks only the final user message, rather than scanning the entire transcript. Append user_msg when the latest user message does not already match it, preserving legitimate repeated utterances in later turns.aieng-synthetic-data/aieng/syn_data/synbench/agents/critic.py-41-43 (1)
41-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
startswith("APPROVE")misclassifies some rejections as approvals.The prompt asks for exactly
APPROVE, but LLM output is free-form. Text such asApproval is not possible: the agent skipped the refund check.orApprove? No — the reply omits the order ID.both start withAPPROVEafterupper(). The critic then accepts a bad draft anddialogue._run_executor_turnskips the retry.Match the approval token exactly, or require it as a standalone first line.
🐛 Proposed fix
- text = (response.content or "").strip() - approved = text.upper().startswith("APPROVE") - return approved, text + text = (response.content or "").strip() + first_line = text.splitlines()[0].strip().rstrip(".!").upper() if text else "" + approved = first_line == "APPROVE" + return approved, text🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/critic.py` around lines 41 - 43, Update the approval parsing in the critic response flow to avoid prefix matches: accept only an exact APPROVE token or APPROVE as a standalone first line, while treating explanatory or negated responses as rejections. Preserve returning the stripped response text alongside the boolean result.aieng-synthetic-data/aieng/syn_data/synbench/agents/single.py-45-46 (1)
45-46: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove the synthetic
role_traceentry.
display.pyrendersrole_traceas role names, and notebook output showsinitial_user_messagebeside actual roles. Removesession.role_trace.append("initial_user_message")so both runners expose consistent role names.🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/single.py` around lines 45 - 46, Remove the synthetic role_trace append from the AgentSession setup in the single-agent runner, leaving session.role_trace populated only by actual roles so its output remains consistent with the other runners.aieng-synthetic-data/aieng/syn_data/synbench/generation/sampler.py-111-116 (1)
111-116: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReport misconfigured
generation.yamlfield names asValueError.Two field lookups trust
generation.yamlwithout checking it againstdb.json:
- Line 111:
record[self.cfg.id_field]raises a bareKeyErrorwhenid_fielddoes not exist on the primary record. The surrounding guards at Lines 98 and 106 raise descriptiveValueError, and the docstring at Lines 91-95 documents onlyValueError.- Line 116:
record.get(rel.via, "")silently substitutes an empty string whenrelated.*.vianames a missing field. That empty id then reaches the generation prompt and the oracle arguments as a valid-looking constraint.The empty-string default is the more damaging case, because it produces plausible but unusable tasks instead of failing.
🛡️ Proposed fix to fail loudly on a bad field mapping
record = self.rng.choice(records) - primary_id = str(record[self.cfg.id_field]) + if self.cfg.id_field not in record: + raise ValueError( + f"generation.yaml id_field '{self.cfg.id_field}' is missing from " + f"db['{self.cfg.primary_collection}'] records" + ) + primary_id = str(record[self.cfg.id_field]) # IDs the LLM must reuse in the draft Task (tools + user message) entities: dict[str, str] = {self.cfg.id_field: primary_id} for key, rel in self.cfg.related.items(): - entities[key] = str(record.get(rel.via, "")) + if rel.via not in record: + raise ValueError( + f"generation.yaml related['{key}'].via '{rel.via}' is missing " + f"from db['{self.cfg.primary_collection}'] records" + ) + entities[key] = str(record[rel.via])🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/generation/sampler.py` around lines 111 - 116, Update the record field lookups in the sampler generation flow: validate that cfg.id_field and every related rel.via field exist on the primary record, raising descriptive ValueError exceptions consistent with the guards around them. Replace the related-field record.get fallback so missing mappings cannot become empty-string entity IDs, while preserving normal ID conversion for valid fields.aieng-synthetic-data/aieng/syn_data/synbench/generation/llm.py-14-24 (1)
14-24: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winEnforce an object result from
complete_json.
ChatClient.complete_jsoncan return a scalar when the response is an array such as[1]or["x"]. A scalar root also produces a generic conversion exception. Reject non-object results with a clear generation error beforegenerator.pyaccessesdata["id"]ordata.get(...).🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/generation/llm.py` around lines 14 - 24, Update call_llm_json to validate the result from client.complete_json before returning it, accepting only dictionary/object results and raising a clear generation error for arrays or scalar roots. Preserve the existing successful object return path so generator.py can safely use data["id"] and data.get(...).
🧹 Nitpick comments (17)
aieng-synthetic-data/tests/synbench/test_schemas_action_compare.py (1)
14-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest
action_fingerprint()with raw actions.Lines 14-16 normalize both inputs before calling
action_fingerprint(). The test will still pass ifaction_fingerprint()stops normalizing its input. Passaandbdirectly to protect the public contract.Proposed fix
- assert action_fingerprint(normalize_action(a)) == action_fingerprint( - normalize_action(b) - ) + assert action_fingerprint(a) == action_fingerprint(b)🤖 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 `@aieng-synthetic-data/tests/synbench/test_schemas_action_compare.py` around lines 14 - 16, Update the test assertion for action_fingerprint() to pass raw actions a and b directly, removing both normalize_action() calls. Keep the comparison focused on action_fingerprint() so the test verifies its public normalization behavior.implementations/agent_benchmark_generation/utils/utils.py (1)
4-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the return type annotation.
The repository runs Ruff. A
Pathreturn annotation documents the contract for the notebook callers that buildDOMAIN_PATHandOUT_DIRfrom this value.♻️ Proposed refactor
-def get_root_directory(): +def get_root_directory() -> Path: """Get the root directory of the agent benchmark generation implementation.""" return Path(__file__).resolve().parents[1]🤖 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 `@implementations/agent_benchmark_generation/utils/utils.py` around lines 4 - 6, Update get_root_directory to add a Path return type annotation, preserving its existing Path return value for callers constructing DOMAIN_PATH and OUT_DIR.implementations/agent_benchmark_generation/README.md (1)
67-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language to the fenced block.
markdownlint reports MD040 for this block. Use
textso the ASCII diagram renders without syntax highlighting. Remove the trailing blank line inside the fence as well.🤖 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 `@implementations/agent_benchmark_generation/README.md` around lines 67 - 80, Update the fenced ASCII diagram in the README to specify the text language for Markdown syntax highlighting, and remove the extra blank line before the closing fence.Source: Linters/SAST tools
implementations/agent_benchmark_generation/1-check_access_to_model.ipynb (1)
28-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the proxy base URL configurable.
The base URL is hardcoded. Users on another OpenAI-compatible endpoint must edit the notebook. Read it from the environment with the current value as the default, so the same
.envfile controls both the key and the endpoint.♻️ Proposed refactor
- "client = OpenAI(base_url=\"https://proxy.vectorinstitute.ai/v1\", api_key=os.environ.get(\"OPENAI_API_KEY\"))\n", + "client = OpenAI(\n", + " base_url=os.environ.get(\"OPENAI_BASE_URL\", \"https://proxy.vectorinstitute.ai/v1\"),\n", + " api_key=os.environ.get(\"OPENAI_API_KEY\"),\n", + ")\n",🤖 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 `@implementations/agent_benchmark_generation/1-check_access_to_model.ipynb` around lines 28 - 38, Update the OpenAI client initialization around OpenAI so base_url is read from an environment variable, using the existing proxy URL as the default; keep OPENAI_API_KEY environment-based so both endpoint and credentials are controlled by the same .env configuration.implementations/agent_benchmark_generation/data/benchmarks/mock_retail/tasks.json (1)
13-27: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
communicate_infois empty whilereward_basisincludesCOMMUNICATE.Tasks
gen_64991457_0(this block) declaresCOMMUNICATEinreward_basisbut supplies no required phrases. The communicate term then passes unconditionally, so the reward reduces to theDBterm alone. That makes the declared reward basis misleading for readers of the benchmark file.Either populate
communicate_infoor omitCOMMUNICATEfromreward_basisfor this task.🤖 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 `@implementations/agent_benchmark_generation/data/benchmarks/mock_retail/tasks.json` around lines 13 - 27, Update the evaluation criteria for task gen_64991457_0 so communicate_info is populated with the required communication phrases, or remove COMMUNICATE from reward_basis; keep the reward basis consistent with the task’s actual communication requirements.aieng-synthetic-data/aieng/syn_data/synbench/agents/prompts.py (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPolicy truncation is silent and uses magic numbers.
Three different hard-coded limits truncate
domain.policy(4000, 2000, 1500 characters). If a domain policy exceeds the limit, the agent never sees the trailing rules, butverification/domain_checks.pystill enforces the full policy. That produces unexplained score drops that look like model failures.Name the limits as module constants and log or warn when truncation occurs.
♻️ Proposed change
+AGENT_POLICY_CHARS = 4000 +PLANNER_POLICY_CHARS = 2000 +CRITIC_POLICY_CHARS = 1500Then reference the constants in each prompt builder instead of the inline literals.
Also applies to: 43-43, 59-59
🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/prompts.py` at line 23, Define named module-level constants for the three policy truncation limits, replace the inline 4000, 2000, and 1500 values in each prompt builder with those constants, and emit a warning or log whenever domain.policy is truncated. Update the prompt-building code around the referenced policy slices while preserving the existing limits and prompt behavior.aieng-synthetic-data/tests/synbench/test_tool_loop.py (1)
36-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe assertions hard-code fixture contents and hide the real precondition.
Three couplings make this test brittle:
domain.seed_tasks[1]selects a task by index. A reordering ofdomains/mock_retail/tasks.seed.jsonsilently changes what is tested.- Lines 40-41 hard-code
get_orderandcancel_order. The stub already replaystask.evaluation_criteria.actions, so the loop can be checked against that list directly.- Line 42 requires
communicate_infoto contain the substringcancel. Ifcommunicate_infois empty, the stub falls back to"Done."on line 22 and the assertion fails with no indication of the cause.Derive the expectations from the task so a fixture change produces a clear failure.
♻️ Proposed change
def test_tool_loop_collects_actions(mock_retail_path): """The loop records each dispatched tool call in order.""" domain = load_domain(mock_retail_path) task = domain.seed_tasks[1] + expected = [a.name for a in task.evaluation_criteria.actions] + assert expected, "seed task must define oracle actions" client = _OracleStubClient(task) session = ToolCallingLoop(domain, client=client).run(task) - assert len(session.agent_actions) == 2 - assert session.agent_actions[0].name == "get_order" - assert session.agent_actions[1].name == "cancel_order" - assert any("cancel" in m.lower() for m in session.agent_messages) + assert [a.name for a in session.agent_actions] == expected + assert session.agent_messages🤖 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 `@aieng-synthetic-data/tests/synbench/test_tool_loop.py` around lines 36 - 42, Update the test around ToolCallingLoop.run to select the seed task by an explicit identifying property rather than index, derive expected action names from task.evaluation_criteria.actions instead of hard-coded get_order/cancel_order values, and assert the communication expectation from the task’s configured criteria while making an empty communicate_info case explicit. Keep the assertions aligned with the task data so fixture changes fail clearly.aieng-synthetic-data/aieng/syn_data/synbench/agents/pipeline.py (2)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
roles or DEFAULT_ROLESshares a module-level mutable list and treats[]as unset.Two effects:
- When the caller passes no
roles,self.rolesis bound to the module-levelDEFAULT_ROLESobject. Any in-place mutation ofpipeline.roleschanges the default for every laterAgentPipeline.roles=[]is falsy, so an explicit empty role list silently enables all four roles.run_user_dialogueinagents/dialogue.pyhas the same behavior on line 42.♻️ Proposed change
-DEFAULT_ROLES = ["user_sim", "planner", "executor", "critic"] +DEFAULT_ROLES: tuple[str, ...] = ("user_sim", "planner", "executor", "critic")- self.roles = roles or DEFAULT_ROLES + self.roles = list(DEFAULT_ROLES) if roles is None else list(roles)Also applies to: 32-32
🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/pipeline.py` at line 17, Update AgentPipeline role initialization to distinguish roles=None from an explicitly empty list and copy DEFAULT_ROLES when defaults are selected, preventing instance mutations from changing the module-level list. Apply the same None-aware, non-shared handling in run_user_dialogue.
17-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
roles or <default>treats an explicit empty list as unset in both role-selection sites. Both files use truthiness to detect a missingrolesargument. A caller that passesroles=[]to disable all roles instead gets the full four-role default, andpipeline.pyadditionally binds the shared module-levelDEFAULT_ROLESlist. Test only forNone.
aieng-synthetic-data/aieng/syn_data/synbench/agents/pipeline.py#L17-L32: makeDEFAULT_ROLESa tuple and setself.roles = list(DEFAULT_ROLES) if roles is None else list(roles).aieng-synthetic-data/aieng/syn_data/synbench/agents/dialogue.py#L42-L42: change toactive = ["user_sim", "planner", "executor", "critic"] if roles is None else roles.🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/pipeline.py` around lines 17 - 32, Preserve an explicitly empty roles list instead of falling back to defaults: in aieng-synthetic-data/aieng/syn_data/synbench/agents/pipeline.py#L17-L32, make DEFAULT_ROLES a tuple and update AgentPipeline.__init__ to copy DEFAULT_ROLES when roles is None, otherwise copy the supplied roles; in aieng-synthetic-data/aieng/syn_data/synbench/agents/dialogue.py#L42-L42, update the active-role selection to check roles is None rather than truthiness, preserving [] as no active roles.aieng-synthetic-data/aieng/syn_data/synbench/agents/planner.py (1)
24-29: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe planner history drops user turns and can break role alternation.
Two concerns in this loop:
- Only
assistantmessages are forwarded. Earlier customer turns are dropped, so the planner cannot see identifiers or constraints the customer already gave. It sees agent replies plus the newest user message only.- The result can contain several consecutive
assistantentries, and the first entry aftersystemis anassistantmessage.UserSimulator._merge_consecutiveinagents/user_sim.pyexists precisely because some providers reject that shape. The planner has no equivalent handling.Forward both
userandassistantturns in order, and skip assistant messages that carrytool_calls, matching the filtering inagents/user_sim.py.Also fix the comment typo:
hostory→history.♻️ Proposed change
- # First append the hostory - for m in session.messages: - if m.get("role") == "assistant" and m.get("content"): - messages.append({"role": "assistant", "content": m["content"]}) - # Then append the user message + # First append the history, keeping both sides of the conversation. + for m in session.messages: + role = m.get("role") + content = (m.get("content") or "").strip() + if not content or role not in ("user", "assistant"): + continue + if role == "assistant" and m.get("tool_calls"): + continue + messages.append({"role": role, "content": content}) + # Then append the current user message. messages.append({"role": "user", "content": user_message})🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/planner.py` around lines 24 - 29, Update the history-building loop in the planner to preserve both user and assistant messages in their original order, excluding assistant messages that contain tool_calls, consistent with UserSimulator filtering. Ensure the resulting messages maintain valid role alternation by applying the existing consecutive-message merge approach or equivalent before appending the new user message, and correct the comment typo from “hostory” to “history”.aieng-synthetic-data/tests/synbench/test_user_sim_perspective.py (1)
102-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe prefix test depends on an unstated fixture value.
UserSimulator._cleanstripsCustomer,User, andsession.task.user_scenario.user_name. This test passes only whendomain.seed_tasks[0].user_scenario.user_nameequalsAlice Chen. Ifdomains/mock_retail/tasks.seed.jsonuses a different name,_cleanleaves the prefix in place and the assertion fails without naming the cause.Build the reply from the task so the test states its own precondition.
♻️ Proposed change
def test_speaker_prefix_is_stripped(mock_retail_path): """A leading speaker name is removed from the simulator's reply.""" domain = load_domain(mock_retail_path) session = _session(domain) - client = _CaptureClient(reply="Alice Chen: it is ord_1001.") + user_name = session.task.user_scenario.user_name + assert user_name, "seed task must define user_name" + client = _CaptureClient(reply=f"{user_name}: it is ord_1001.") assert UserSimulator(client).respond(session) == "it is ord_1001."🤖 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 `@aieng-synthetic-data/tests/synbench/test_user_sim_perspective.py` around lines 102 - 108, Update test_speaker_prefix_is_stripped to derive the speaker name from the loaded task’s user_scenario.user_name, use that value when constructing the captured reply, and retain the assertion for the stripped response so the test explicitly matches UserSimulator._clean behavior without relying on a hardcoded fixture name.aieng-synthetic-data/aieng/syn_data/synbench/agents/loop.py (2)
100-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTool dispatch failures are swallowed without a log.
The bare
except Exceptionconverts every dispatch failure into a JSON error string in the transcript. That is reasonable for agent sandboxing, because the model should see the error and recover. However, nothing records the failure outside the transcript. A schema mistake indomains/mock_retail/tools.pyor a bug inenvironment/core.pypresents itself as an agent-side quality problem.Add a debug or warning log next to the handler and keep the transcript payload unchanged.
Note: the
json.dumpsstatic analysis hints on lines 86, 101, and 103 do not apply. These strings are OpenAI tool-call payloads, not HTTP responses.🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/loop.py` around lines 100 - 104, Update the dispatch exception handler in the agent loop around env.dispatch to emit a debug or warning log containing the caught exception, while preserving the existing JSON error string in result_str unchanged. Do not alter the tool-call payload serialization.Source: Linters/SAST tools
71-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTurn-budget exhaustion ends the loop with no signal.
If the model requests tool calls on every one of
max_turnsiterations, the loop exits through thecontinuepath. No final assistant text is recorded, andsession.agent_messagesstays empty.score_trajectoryinevaluation/scoring.pythen evaluates a trajectory that has no communication output, which is indistinguishable from a model that simply refused to answer.The same silence applies when a response carries neither
tool_callsnorcontent: line 122 breaks without recording anything.Record the termination reason on the session, or emit a warning log, so evaluation runs can separate a budget overrun from a genuine agent failure.
🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/loop.py` around lines 71 - 122, Add explicit termination signaling to the loop around the response handling in the agent method: record whether the loop ended because max_turns was exhausted after repeated tool calls, and record a distinct reason when a response has neither tool_calls nor content. Store the reason on the session using its existing metadata/state mechanism, or emit a warning through the established logger, so score_trajectory can distinguish budget exhaustion and empty responses from genuine refusal.aieng-synthetic-data/aieng/syn_data/synbench/agents/llm_agent.py (1)
10-12: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueUse
model_dump(mode="json")for serialization safety.
Action.argumentsaccepts arbitrary values. Python-mode output can causejson.dumpsto raiseTypeErrorfor values such asdatetime,Decimal, orUUID.♻️ Proposed change
- return json.dumps([a.model_dump() for a in actions]) + return json.dumps([a.model_dump(mode="json") for a in actions])🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/agents/llm_agent.py` around lines 10 - 12, Update actions_to_json to call Action.model_dump with mode="json" before passing the results to json.dumps, ensuring arbitrary Action.arguments values such as datetime, Decimal, and UUID are converted into JSON-safe representations.Source: Linters/SAST tools
aieng-synthetic-data/aieng/syn_data/synbench/generation/prompt.py (2)
41-45: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDerive
write_rulefrom the FSM tags, not the path length.
n > 1is a proxy for "the path contains a write step". A task type withallow_write: trueand a single-element path takes theelsebranch and receives "do not call write tools", which contradicts its own config.No current
mock_retailtask type hits this combination, so this is a robustness improvement rather than an active defect. Checking the path tags removes the coupling to path length.♻️ Proposed refactor
+ write_tags = { + t.fsm_tag for t in domain.tools if t.tool_type == ToolType.WRITE + } write_rule = ( "Include exactly one write tool as the final action." - if allow_write and n > 1 + if allow_write and any(tag in write_tags for tag in path) else "Use read-only tools only; do not call write tools." )🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/generation/prompt.py` around lines 41 - 45, Derive write_rule from the FSM path tags rather than the path length: update the logic near write_rule to detect whether the path contains a write step, while still requiring allow_write. Ensure single-element paths with a write tag receive the final-write instruction, and read-only paths retain the no-write instruction.
203-212: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake policy truncation explicit.
When a domain policy exceeds 1,500 characters, the prompt silently drops the remainder. Add a named limit with a comment that explains the prompt-budget reason, and append a truncation marker when the policy is incomplete.
🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/generation/prompt.py` around lines 203 - 212, Update the prompt construction in the generation function containing the domain policy to define a named 1,500-character policy limit with a comment explaining the prompt-budget constraint. Detect policies exceeding that limit, truncate them, and append a clear truncation marker; leave shorter policies unchanged while preserving the existing prompt structure.aieng-synthetic-data/aieng/syn_data/synbench/generation/generator.py (1)
58-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the verification post-processing.
run_and_verify(Lines 60-64) andverify_drafts(Lines 70-73) contain identicalfilter_verifiedhandling. Letrun_and_verifydelegate toverify_drafts.♻️ Proposed refactor
def run_and_verify(self, n: int) -> tuple[list[Task], list[tuple[Task, list[str]]]]: """Generate ``n`` drafts and split them into verified tasks and rejections.""" - drafts = self.run(n) - verified, rejected_drafts, _ = filter_verified(self.domain, drafts) - rejections = [(r.task, errors) for r, errors in rejected_drafts] - verified_tasks = [v.task for v in verified] - return verified_tasks, rejections + return self.verify_drafts(self.run(n))🤖 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 `@aieng-synthetic-data/aieng/syn_data/synbench/generation/generator.py` around lines 58 - 73, Deduplicate the verification handling by updating run_and_verify to generate drafts with run(n) and return the result of verify_drafts(drafts). Keep the existing filter_verified processing and return behavior centralized in verify_drafts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 078e75bb-d93d-45b7-adca-ed47d5ce9dd1
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (96)
.coderabbit.yaml.pre-commit-config.yamlaieng-synthetic-data/aieng/syn_data/image/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/critic.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/dialogue.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/llm_agent.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/loop.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/pipeline.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/planner.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/prompts.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/session.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/single.pyaieng-synthetic-data/aieng/syn_data/synbench/agents/user_sim.pyaieng-synthetic-data/aieng/syn_data/synbench/display.pyaieng-synthetic-data/aieng/syn_data/synbench/domain/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/domain/loader.pyaieng-synthetic-data/aieng/syn_data/synbench/environment/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/environment/communicate.pyaieng-synthetic-data/aieng/syn_data/synbench/environment/core.pyaieng-synthetic-data/aieng/syn_data/synbench/environment/hashing.pyaieng-synthetic-data/aieng/syn_data/synbench/evaluation/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/evaluation/metrics.pyaieng-synthetic-data/aieng/syn_data/synbench/evaluation/scoring.pyaieng-synthetic-data/aieng/syn_data/synbench/fsm/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/fsm/validator.pyaieng-synthetic-data/aieng/syn_data/synbench/generation/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/generation/generator.pyaieng-synthetic-data/aieng/syn_data/synbench/generation/llm.pyaieng-synthetic-data/aieng/syn_data/synbench/generation/prompt.pyaieng-synthetic-data/aieng/syn_data/synbench/generation/sampler.pyaieng-synthetic-data/aieng/syn_data/synbench/llm/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/llm/chat_client.pyaieng-synthetic-data/aieng/syn_data/synbench/llm/client.pyaieng-synthetic-data/aieng/syn_data/synbench/llm/config.pyaieng-synthetic-data/aieng/syn_data/synbench/llm/parsing.pyaieng-synthetic-data/aieng/syn_data/synbench/llm/tools.pyaieng-synthetic-data/aieng/syn_data/synbench/schemas/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/schemas/actions.pyaieng-synthetic-data/aieng/syn_data/synbench/schemas/domain.pyaieng-synthetic-data/aieng/syn_data/synbench/schemas/generation.pyaieng-synthetic-data/aieng/syn_data/synbench/schemas/tasks.pyaieng-synthetic-data/aieng/syn_data/synbench/schemas/tools.pyaieng-synthetic-data/aieng/syn_data/synbench/schemas/verification.pyaieng-synthetic-data/aieng/syn_data/synbench/verification/__init__.pyaieng-synthetic-data/aieng/syn_data/synbench/verification/domain_checks.pyaieng-synthetic-data/aieng/syn_data/synbench/verification/errors.pyaieng-synthetic-data/aieng/syn_data/synbench/verification/pipeline.pyaieng-synthetic-data/pyproject.tomlaieng-synthetic-data/tests/synbench/__init__.pyaieng-synthetic-data/tests/synbench/conftest.pyaieng-synthetic-data/tests/synbench/fixtures/llm_responses/draft_cancel.jsonaieng-synthetic-data/tests/synbench/fixtures/tasks_invalid_fsm.jsonaieng-synthetic-data/tests/synbench/fixtures/tasks_invalid_replay.jsonaieng-synthetic-data/tests/synbench/fixtures/tasks_valid.jsonaieng-synthetic-data/tests/synbench/integration/__init__.pyaieng-synthetic-data/tests/synbench/integration/conftest.pyaieng-synthetic-data/tests/synbench/integration/test_agentic_pipeline.pyaieng-synthetic-data/tests/synbench/integration/test_call_llm_json.pyaieng-synthetic-data/tests/synbench/integration/test_generation_run.pyaieng-synthetic-data/tests/synbench/test_environment_tool_dispatch.pyaieng-synthetic-data/tests/synbench/test_evaluation_metrics.pyaieng-synthetic-data/tests/synbench/test_fsm_invalid_path.pyaieng-synthetic-data/tests/synbench/test_fsm_valid_path.pyaieng-synthetic-data/tests/synbench/test_generation_config.pyaieng-synthetic-data/tests/synbench/test_generation_dedup.pyaieng-synthetic-data/tests/synbench/test_prompt_builder_snapshot.pyaieng-synthetic-data/tests/synbench/test_schemas_action_compare.pyaieng-synthetic-data/tests/synbench/test_schemas_task_roundtrip.pyaieng-synthetic-data/tests/synbench/test_tool_loop.pyaieng-synthetic-data/tests/synbench/test_user_sim_perspective.pyaieng-synthetic-data/tests/synbench/test_verifier_policy_rules.pyaieng-synthetic-data/tests/synbench/test_verifier_replay_failure.pyaieng-synthetic-data/tests/synbench/test_verifier_replay_success.pyaieng-synthetic-data/tests/test_smoke.pyimplementations/agent_benchmark_generation/1-check_access_to_model.ipynbimplementations/agent_benchmark_generation/2-generate_and_verify_tasks.ipynbimplementations/agent_benchmark_generation/3-single_agent_evaluation.ipynbimplementations/agent_benchmark_generation/4-multi_agent_pipeline_evaluation.ipynbimplementations/agent_benchmark_generation/README.mdimplementations/agent_benchmark_generation/__init__.pyimplementations/agent_benchmark_generation/data/benchmarks/mock_retail/tasks.jsonimplementations/agent_benchmark_generation/domains/mock_retail/db.jsonimplementations/agent_benchmark_generation/domains/mock_retail/generation.yamlimplementations/agent_benchmark_generation/domains/mock_retail/policy.mdimplementations/agent_benchmark_generation/domains/mock_retail/state_machine.yamlimplementations/agent_benchmark_generation/domains/mock_retail/tasks.seed.jsonimplementations/agent_benchmark_generation/domains/mock_retail/tools.pyimplementations/agent_benchmark_generation/domains/mock_retail/user_simulator.yamlimplementations/agent_benchmark_generation/domains/mock_retail/verify.pyimplementations/agent_benchmark_generation/utils/__init__.pyimplementations/agent_benchmark_generation/utils/utils.pyimplementations/implementation_b/README.mdimplementations/implementation_b/topic_b_a.ipynbpyproject.toml
💤 Files with no reviewable changes (4)
- aieng-synthetic-data/aieng/syn_data/image/init.py
- implementations/implementation_b/topic_b_a.ipynb
- aieng-synthetic-data/tests/test_smoke.py
- implementations/implementation_b/README.md
There was a problem hiding this comment.
Cell #1 I get errors for from utils.utils import get_root_directory.
The kernel is running from the repo root, not from implementations/agent_benchmark_generation, so from utils.utils import ... fails.
The notebook assumes cwd = its own directory, but Cursor/VS Code (and many other runners) default to the workspace root.
There was a problem hiding this comment.
Hmm, I see what you mean. My Jupyter kernel is also set to the default .venv/bin/activate. The notebooks also run on Coder with no errors, so that's why I assumed it should be fine. Let's meet quickly whenever you are available so that I can find out what I am doing differently.
I also just looked into another reference implementation to see how they handle utils. It seems they are doing a similar thing and import utils in notebooks ( Example: from utils.dataset_helpers), but I might definitely be missing something, so let's meet.
There was a problem hiding this comment.
Please use rich to format cell print outputs (tables, panels, etc), they help a lot in readability. E.g. in step 4 I see a sample json but it's a bit hard to figure out what is what.
There was a problem hiding this comment.
are Run and Draft jargons defined in τ-bench? Can you provide a brief explanation about them?
I think having a workflow diagram of the steps would help clarify what the notebooks do. Maybe we could have one for the overall notebooks workflow, and then a separate diagram for each notebook if it makes sense.
There was a problem hiding this comment.
I noticed that when we change ROOT to Implementation root: /../synthetic-data-bootcamp step 6 fails here:
assert errors == [], f"Domain validation failed: {errors}"
So I think we should update cell 2 like this:
DOMAIN_PATH = ROOT / "implementations" / "agent_benchmark_generation" / "domains" / "mock_retail"
OUT_DIR = ROOT / "implementations" / "agent_benchmark_generation" / "data" / "benchmarks" / "mock_retail"
There was a problem hiding this comment.
nit: task.json is not added to gitignore.
There was a problem hiding this comment.
In notebook 4 - step 2:
task_path = OUT_DIR / "tasks.json"
if it's the same file we generated and used in previous notebooks, it's under ROOT / "data" / "benchmarks" / "mock_retail" and current value of OUT_DIR results in file not found error because "notebook_walkthrough" is empty.
| @@ -1,5 +1,5 @@ | |||
| [project] | |||
| name = "aieng-synthetic-data" | |||
There was a problem hiding this comment.
Changed this according to the repo level name
| ] | ||
|
|
||
| [project] | ||
| name = "aieng-topic-impl" |
There was a problem hiding this comment.
Set this to the package name
There was a problem hiding this comment.
Configurations for an improved coderabbit setup
| python-version-file: ".python-version" | ||
|
|
||
| - name: Install dependencies and check code | ||
| # Install dependencies for all the necessary groups |
There was a problem hiding this comment.
TODO: remove this comment
| uv sync --all-extras --dev | ||
| uv run --directory aieng-synthetic-data pytest -m "not integration_test" tests | ||
| uv run --directory aieng-synthetic-data pytest -m "not integration_test" --ignore=tests/synbench tests | ||
| # Run synbench tests if there are changes in the synbench package |
There was a problem hiding this comment.
Added this job to run synbench tests only when there are changes under 'aieng-synthetic-data/aieng/synbench/**
Otherwise, synbench tests won't run.
| drafts: list[Task] = [] | ||
| for i in range(n): | ||
| tid = f"gen_{uuid4().hex[:8]}_{i}" | ||
| drafts.append(self.generator.generate_one(task_id=tid)) |
There was a problem hiding this comment.
For follow-up PRs and speed-up:
We can call some of the generations simultaneously with multi-threading and semaphores to speed up this part.
2 participants
Summary
Clickup Ticket(s): Link(s) if applicable.
https://app.clickup.com/t/10524786/868kerrr8
https://app.clickup.com/t/10524786/868kerrd0
Type of Change
Changes Made
Testing
uv run pytest tests/)uv run mypy <src_dir>)uv run ruff check src_dir/)Manual testing is performed by running notebooks and extensively inspecting the steps.
Manual testing details:
Screenshots/Recordings
Related Issues
Deployment Notes
Checklist
Summary by CodeRabbit
New Features
Documentation
Tests