Add an agent in production example #3890
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
TL;DR: I like the high-level idea (that you demoed to us previously), but I hope we can remove as much of the non-critical complexity to make the ZenML parts shine a bit more?
Duplication
Seems like there's quite a bit of duplication between llm_utils.py and steps/analyze.py (both have LLM availability checking functions, both implement document analysis check, text cleaning and extraction functions are duplicated etc). Seems also like llm_utils.py is mostly unused in the actual pipeline, but I might be wrong about that? Also steps/utils.py has some duplication?
llm_utils.py vs steps/utils.py have duplicate stop words / filter lists (though not fully overlapping?
steps/render.py and steps/evaluate.py and app/main.py have a bunch of inline CSS which probably could be consolidated?
There are two different definitions of DocumentRequest: in models.py and in app/main.py. This is confusing.
Showcasing ZenML
Where poss we should try to pull out the HTML / CSS code into separate files / modules as I think it gets in the way of understanding what ZenML is doing.
Overly complicated stuff
The multiprocessing stuff in app/main.py feels like it's a bit complex. Maybe rather just two endpionts: POST /analyze which submits pipeline and return run_id, and then GET /result?run_id=... which polls until the artifact is ready. Removes a TON of probably unnecessary complexity which doesn't really add to the learning experience for a user / reader of the guide.
There's a bunch of places where the try...except pattern is done a bit too much, maybe to the point where you are actually smoothing over errors that should get raised? e.g. In steps/analyze.py, the analyze_document_step has a try...except block that calls perform_llm_analysis. Inside perform_llm_analysis, there is another try...except block that catches a failure and then re-raises it.
Under-complicated stuff
Wondering if it's too simplistic to be called an 'agent'. Maybe it doesn't really matter to showcase the pattern, but I guess it's stretching the limits of the word 'agent'.
Random stuff
lots of print() statements. Makes the output quite verbose. Either make them debug logging statements? (Also perhaps in general prefer logging over print?
Remove langfuse? It's mentioned a few times but it's never really used anywhere so it just adds confusion (and an extra dependency).
Feels like we might have wanted to use the model control plane to unify some of this behaviour / logic etc, but also maybe we don't want to foreground that so much?
| try: | ||
| # Try different ways to access the output | ||
| if hasattr(step, "output") and step.output: | ||
| print("Trying step.output") | ||
| analysis_result = step.output.load() | ||
| print( | ||
| f"Successfully loaded from step.output: {type(analysis_result)}" | ||
| ) | ||
| elif hasattr(step.outputs, "items"): | ||
| print("Trying step.outputs.items()") | ||
| for ( | ||
| output_name, | ||
| output_artifact, | ||
| ) in step.outputs.items(): | ||
| print( | ||
| f"Trying to load output: {output_name}, type: {type(output_artifact)}" | ||
| ) | ||
| analysis_result = ( | ||
| output_artifact.load() # type: ignore | ||
| ) | ||
| print( | ||
| f"Successfully loaded: {type(analysis_result)}" | ||
| ) | ||
| break | ||
| elif isinstance(step.outputs, dict): | ||
| print( | ||
| "Outputs is dict, getting first value" | ||
| ) | ||
| first_output = next( | ||
| iter(step.outputs.values()) | ||
| ) | ||
| analysis_result = first_output.load() # type: ignore | ||
| print( | ||
| f"Successfully loaded from first output: {type(analysis_result)}" | ||
| ) | ||
| else: | ||
| print( | ||
| f"Unknown outputs structure: {type(step.outputs)}" | ||
| ) |
There was a problem hiding this comment.
This stuff feels a bit ugly. Probably a nicer way to handle this.
| model_version = client.get_model_version( | ||
| "document_analysis_agent", "production" | ||
| ) |
There was a problem hiding this comment.
We sort of just assume that this exists already?
There was a problem hiding this comment.
yeah i think its a pre-req
| ) | ||
|
|
||
| # Run the pipeline in a separate process to avoid signal handling in worker thread | ||
| tmp_dir = tempfile.mkdtemp(prefix="doc_analysis_") |
There was a problem hiding this comment.
we never actually clean this up at the end, I think... maybe do that?
| def perform_llm_analysis( | ||
| content: str, | ||
| filename: str, | ||
| model: str = "gpt-3.5-turbo", |
There was a problem hiding this comment.
maybe 4o-mini instead? 3.5 turbo might actually be more expensive, even, and much less capable. or use one of the 5-mini or 5-nano to make the example a bit more future-forward?
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=["*"], | ||
| allow_credentials=True, | ||
| allow_methods=["*"], | ||
| allow_headers=["*"], | ||
| ) |
There was a problem hiding this comment.
maybe should add a comment that this is dev example only or something, otherwise maybe just a bit too permissive?
| raise | ||
|
|
||
|
|
||
| def perform_deterministic_analysis( |
There was a problem hiding this comment.
Not sure if we ever want to be doing this? Consider removing? Feels like unnecessary complexity.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
✅ No broken links found! |
Documentation Link Check Results✅ Absolute links check passed |
|
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 29.5%, saving 396.72 KB.
368 images did not require optimisation. Update required: Update image-actions configuration to the latest version before 1/1/21. See README for instructions. |
* Add ZenML introduction to README.md * added agent example * Update evaluation pipeline max items to 100, 125, and 125 * Add document analysis pipeline and evaluation pipeline * Code review * Simplify analysis result extraction from artifacts * Refactor artifact loading and handling logic * Add "Your First AI Pipeline" section to table of contents * Add your first AI pipeline example for ZenML * Add login command for ZenML dashboard * Update Docker command for container startup * Update Docker compose command to start containers * Add architecture overview and screenshots * Add figures for pipeline visualization in ZenML dashboard * Add reasons for using pipelines in AI/ML workflows * Revise AI pipeline benefits in quickstart guide * Update AI pipeline architecture diagram layout * Update flowchart orientation to top to bottom * Add architecture overview in getting started guide * Add example installation instructions and architecture overview * Add whitespace and update comments for clarity * Update AI pipeline and examples with new links and services * Update ZenML hello-world.md with login step & new links.- * Refactor quickstart README for clarity and consistency * Add FloraCast and Retail Forecast to projects list * Update mypy configuration to ignore missing imports * Update ignore_missing_imports setting in pyproject.toml * Update links in README and fix relative path in docs * Add initial pipeline analysis and evaluation diagrams * Images * Delete test.html and add architecture.png image.<commit message> * Optimised images with calibre/image-actions * Update Argilla documentation links to correct URLs * Update docs/book/getting-started/your-first-ai-pipeline.md * Update docs/book/getting-started/your-first-ai-pipeline.md * Add new ML pipeline examples to README.md * Update README with new example images and links * Add example-06 and example-07 images to user guide * Add ZenML Pro cover image --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Alex Strick van Linschoten <[email protected]> (cherry picked from commit bdd3319)
Describe changes
What’s this PR about?
TL;DR version:
Adds a ready-to-use “document analysis agent” showing how ZenML pipelines can integrate with a FastAPI service and UI—plus simple docs to walk you through it.
Adds a production-ready example to the ZenML project that demonstrates how to build a document‑analysis agent using FastAPI, ZenML pipelines, and a simple UI. – [+3361 / −26, 39 files updated] ([GitHub][1])
New in the PR:
Example app (
examples/minimal_agent_production/) featuring:Docs updates:
Project config tweak:
Why it matters for newcomers:
This gives you a plug‑and‑play template—showcasing how to connect ZenML pipelines, an agentic workflow, and a web UI in one cohesive example.
Pre-requisites
Please ensure you have done the following:
developand the open PR is targetingdevelop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes