Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive set of component tests for the flow compiler, covering async sequential and ReAct loop patterns. The tests are well-structured and validate parsing, compilation, and execution. I've provided a few suggestions to improve code consistency, readability, and reduce duplication in the new test file, aligning with established practices for test maintainability.
… 1flmd6) Add 6 component tests covering async sequential flow (ADK LLM Auditor pattern) and ReAct while-loop pattern: - test_parse_sequential_async: verify parser detects async and extracts ActorCall IR - test_compile_sequential_async: verify start/end routers with critic+reviser - test_execute_sequential_async: verify VFS route output matches expected actors - test_compile_react_loop: verify 4 routers (start, if, loop_back, end) - test_execute_react_loop_no_tools: verify conditional skips execute-tool - test_execute_react_loop_with_tools: verify conditional routes to execute-tool
- Use pathlib instead of os.path in VFS helpers for consistency - Use next() instead of list comprehension + [0] for first-match lookups - Extract shared setup into _run_react_loop_if_router helper - Remove unused os import
Add flow-compiler to the component-tests matrix in ci.yml so test_adk_llm_auditor.py (and other flow-compiler tests) run in CI. - Add test-one target to flow-compiler Makefile as CI-compatible alias - Add matrix entry with dummy config (pure Python, no Docker needed)
Add adk_llm_auditor.py - a realistic async agentic flow that exercises ALL flow DSL syntax capabilities in a single flow: - async def + await, class instantiation + method calls - while True loop with break, continue, and max-iteration guard - try-except for resilient LLM generation - fan-out: parallel scoring by two independent LLMs (list literal) - if/elif/else with nested conditionals - early return guard clause, payload mutations Compiles to 20 routers. Add 9 new tests (15 total in file) covering: - IR parsing: all node types present (ActorCall, WhileLoop, FanOutCall, TryExcept, Condition, Break, Continue, Mutation, Return) - Compilation: router count, structure, valid Python output - Execution: init mutations, no-claims exit, loop entry, approved/ standard-revision/deep-revision/continue-marginal decision branches
The adk_llm_auditor flow uses while-True loops, which trigger the pre-existing DOT visualization bug where body actors render as dead-end nodes without edges back to the loop_back router.
atemate
force-pushed
the
1c8d/1flmd6.test-adk-llm-auditor-compilation-execution-test
branch
from
69f376b to
5c645b2
Compare
… optimizations After merging main, the compiler now inlines mutations into start routers and uses VFS-based routing. Update tests to match: - test_routing_correctness: rewrite to use VFS temp dirs instead of message-based routing, set env vars before exec for resolve() - test_adk_llm_auditor: test start_llm_auditor instead of removed router_llm_auditor_line_29_seq - test_dotgen: update color assertions (darkseagreen4/indianred4), fix truncation test for unicode ellipsis, replace _center_text tests with _truncate_display_name - test_complex_flows: mutations_only.py now compiles to 2 routers - Recompile adk_llm_auditor example with updated compiler
The CI graphviz installation may lack PNG rendering libraries (libgd/cairo). The generate_plot tests now gracefully handle RuntimeError from dot -Tpng and fall back to DOT file assertions.
The test_sla_precheck_stops_retries test fails intermittently in CI due to SQS latency jitter with a tight 5s deadline. Increasing to 8s gives 3s more headroom while still testing the same behavior: SLA expiry stops retries before max_attempts (4 of 5 attempts execute).
atemate
deleted the
1c8d/1flmd6.test-adk-llm-auditor-compilation-execution-test
branch
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
1 participant
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.
Summary
1c8d/1flmd6Tests Added
test_parse_sequential_asyncasync def, extracts 2ActorCallIR nodes (critic, reviser)test_compile_sequential_asyncstart_llm_auditor_flow/end_llm_auditor_flowwith correct actor routingtest_execute_sequential_async["critic", "reviser"]to VFSroute/nexttest_compile_react_loop_if),loop_back, endtest_execute_react_loop_no_toolsexecute-toolwhentool_callsis emptytest_execute_react_loop_with_toolsexecute-toolwhentool_callspresentTest plan
make -C testing/component/flow-compiler test -k test_adk)test_dotgenandtest_routing_correctness)