-
Notifications
You must be signed in to change notification settings - Fork 51
Add seizure workshop #203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add seizure workshop #203
Conversation
git-subtree-dir: seizure git-subtree-split: 2c25e7b5e562a30a04d3e1ab6aefde40a44ce353
Summary of ChangesHello @vineet-reddy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new workshop that brings AI capabilities to clinical decision-making. It establishes a multi-agent system designed to interpret clinical notes and provide International League Against Epilepsy (ILAE) seizure outcome scores, aiming to emulate epileptologist reasoning. The integration provides a practical example of applying advanced AI models and agent frameworks to complex medical data, complete with a user-friendly interface and deployment instructions. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a new 'seizure' workshop, which is a multi-agent system for clinical reasoning. The implementation uses Google's ADK and a Streamlit frontend. The code is well-structured, but I've provided several suggestions to improve maintainability, efficiency, and correctness. Key areas for improvement include optimizing the Dockerfile for better caching, correcting the setup instructions in the README, improving dependency management, and making the code more efficient by avoiding object recreation in loops. I've also pointed out some inconsistencies in the Gemini model versions used and suggested strengthening the tests with more specific assertions.
| git clone https://github.com/vineet-reddy/SeizureScoreAI.git | ||
| cd SeizureScoreAI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The installation instructions reference cloning the original source repository. Since this project is now part of the google-gemini/workshops repository, these instructions should be updated to reflect that. Users should clone the workshops repository and then navigate to the seizure directory.
| git clone https://github.com/vineet-reddy/SeizureScoreAI.git | |
| cd SeizureScoreAI | |
| git clone https://github.com/google-gemini/workshops.git | |
| cd workshops/seizure |
| # Copy all code first (needed for setup.py) | ||
| COPY . . | ||
|
|
||
| # Install requirements and local package | ||
| RUN pip install --no-cache-dir -r requirements.txt && pip install --no-cache-dir . | ||
|
|
||
| # Set port for Cloud Run | ||
| ENV PORT=8080 | ||
| EXPOSE 8080 | ||
|
|
||
| # Start Streamlit | ||
| CMD ["bash", "-lc", "streamlit run app/streamlit_app.py --server.port $PORT --server.address 0.0.0.0"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Dockerfile can be optimized for better build performance and clarity.
- Layer Caching: By copying only dependency-related files first (
requirements.txt,setup.py,src), you can leverage Docker's layer caching. This prevents re-installing all Python packages on every code change, speeding up the build process significantly. - CMD Simplification: The
CMDinstruction can be simplified.bash -lcis likely unnecessary in this minimal container. Usingsh -cis more standard and lightweight for shell expansion of environment variables like$PORT.
# Copy dependency files and install dependencies to leverage Docker layer caching
COPY requirements.txt setup.py ./
COPY src ./src
RUN pip install --no-cache-dir -r requirements.txt && pip install --no-cache-dir .
# Copy the rest of the application code
COPY . .
# Set port for Cloud Run
ENV PORT=8080
EXPOSE 8080
# Start Streamlit
CMD ["sh", "-c", "streamlit run app/streamlit_app.py --server.port $PORT --server.address 0.0.0.0"]
| &:hover {{ | ||
| transform: translateY(-2px); | ||
| box-shadow: 0 4px 8px rgba(0,0,0,0.15); | ||
| }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # Core dependencies | ||
| streamlit>=1.40.0 | ||
| python-dotenv>=1.0.0 | ||
| pillow>=10.0.0 | ||
|
|
||
| # Google Agent Development Kit (ADK) | ||
| google-adk>=0.3.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is redundancy in dependency management between requirements.txt and setup.py. To improve maintainability, setup.py should define the dependencies for the seizure_score_ai library, and this file should define dependencies for the application, including the local library itself.
I've also suggested a corresponding change in setup.py. With these changes, the Dockerfile could be simplified to just RUN pip install --no-cache-dir -r requirements.txt.
# Core application dependencies
streamlit>=1.40.0
pillow>=10.0.0
# Install the local seizure_score_ai package and its dependencies
.
| def generate_clinic_note() -> Optional[str]: | ||
| try: | ||
| client = setup_client() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generate_clinic_note function creates a new genai.Client on every call, which is inefficient when called in a loop. It's better to create the client once and pass it as an argument to this function.
| def generate_clinic_note() -> Optional[str]: | |
| try: | |
| client = setup_client() | |
| def generate_clinic_note(client: genai.Client) -> Optional[str]: | |
| try: | |
| # client is now passed as an argument |
| def main(): | ||
| num_notes = 10 # Number of clinic notes to generate | ||
|
|
||
| try: | ||
| setup_client() | ||
| except ValueError as e: | ||
| print(f"Setup Error: {str(e)}") | ||
| return | ||
|
|
||
| for i in range(num_notes): | ||
| print(f"\nGenerating clinic note {i+1}...") | ||
| note = generate_clinic_note() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The genai.Client should be initialized once outside the loop and passed to generate_clinic_note. This avoids creating a new client on each iteration, which is more efficient. This change corresponds to the suggested change in the generate_clinic_note function.
| def main(): | |
| num_notes = 10 # Number of clinic notes to generate | |
| try: | |
| setup_client() | |
| except ValueError as e: | |
| print(f"Setup Error: {str(e)}") | |
| return | |
| for i in range(num_notes): | |
| print(f"\nGenerating clinic note {i+1}...") | |
| note = generate_clinic_note() | |
| def main(): | |
| num_notes = 10 # Number of clinic notes to generate | |
| try: | |
| client = setup_client() | |
| except ValueError as e: | |
| print(f"Setup Error: {str(e)}") | |
| return | |
| for i in range(num_notes): | |
| print(f"\nGenerating clinic note {i+1}...") | |
| note = generate_clinic_note(client) | |
| install_requires=[ | ||
| "streamlit>=1.40.0", | ||
| "google-adk>=0.3.0", | ||
| "python-dotenv>=1.0.0", | ||
| "pillow>=10.0.0", | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is redundancy in dependency management between requirements.txt and setup.py. This file should only list the direct dependencies of the seizure_score_ai package. Application-level dependencies like streamlit should be in requirements.txt. I've made a corresponding suggestion for requirements.txt.
install_requires=[
"google-adk>=0.3.0",
"python-dotenv>=1.0.0",
],| def create_clinical_extractor_agent() -> LlmAgent: | ||
| """Creates the Clinical Information Extractor agent.""" | ||
|
|
||
| instruction = """You are a clinical information extractor. Extract these entities from the clinical note: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This large instruction string is defined inside the create_clinical_extractor_agent function. This pattern is repeated for other agent creation functions in this file. Defining these large, static strings as module-level constants (e.g., _EXTRACTOR_INSTRUCTION = """...""") would improve readability and make it clear they don't change between calls.
| final, detailed = process_clinical_note(clinical_note) | ||
|
|
||
| # Verify we got results | ||
| assert 'ilae_score' in final, "Missing ILAE score" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assertion only checks for the key's presence. To make the test more robust, you should also assert the expected value. For this test case, the ILAE score should be '1'.
| assert 'ilae_score' in final, "Missing ILAE score" | |
| assert final.get('ilae_score') == '1', f"Expected ILAE score '1', but got {final.get('ilae_score')}" |
| print("\nTesting Gemini 2.5 Flash API...") | ||
| try: | ||
| response = client.models.generate_content( | ||
| model="gemini-2.5-flash", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an inconsistency in the Gemini model version used across the project. The main application (agents.py) and the README.md specify gemini-3-flash-preview, but this test uses gemini-2.5-flash. To ensure the test verifies the same model used in production, it's recommended to update this to gemini-3-flash-preview.
| model="gemini-2.5-flash", | |
| model="gemini-3-flash-preview", |
Adds a new workshop in 'seizure' by vendoring https://github.com/vineet-reddy/SeizureScoreAI as a squashed subtree. Source repo: https://github.com/vineet-reddy/SeizureScoreAI