Skip to content

Fix make ty#29

Open
gabemontero wants to merge 2 commits into
redhat-ai-dev:mainfrom
gabemontero:fix-make-ty
Open

Fix make ty#29
gabemontero wants to merge 2 commits into
redhat-ai-dev:mainfrom
gabemontero:fix-make-ty

Conversation

@gabemontero

@gabemontero gabemontero commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?:

Replace type: ignore comments with proper casts and type annotations:

  • ingest.py: cast duck-typed objects to ContentFile
  • methods.py: annotate MCP tool dicts as Any
  • workflow.py: cast messages, file_search_tool, and StateGraph args
  • streamlit_app.py: fix workflow parameter type from Workflow to Any

also updated the README to deal with the pyproject differences between launching llamastack and launching the app

Which issue(s) this PR fixes:

errors seen in #28

PR acceptance criteria:

Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened and linked to this PR, if they are not in the PR scope due to various constraints.

  • [/] Tested and Verified

  • [/] Documentation (READMEs, Product Docs, Blogs, Education Modules, etc.)

How to test changes / Special notes to the reviewer:

gabemontero and others added 2 commits July 10, 2026 13:40
Replace type: ignore comments with proper casts and type annotations:
- ingest.py: cast duck-typed objects to ContentFile
- methods.py: annotate MCP tool dicts as Any
- workflow.py: cast messages, file_search_tool, and StateGraph args
- streamlit_app.py: fix workflow parameter type from Workflow to Any

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: gabemontero <gmontero@redhat.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix ty type-check errors and document pyproject swap for app/server

🐞 Bug fix 📝 Documentation ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Replace type: ignore directives with explicit casts/Any to satisfy ty.
• Clarify README steps to swap pyproject.toml between Llama Stack and the Streamlit app.
• Add pyproject.orig.toml to preserve the app dependency set separately from server/container
 deps.
Diagram

graph TD
  A["Streamlit UI"] --> B["Workflow (LangGraph)"] --> C["Agent methods"] --> D["Llama Stack / OpenAI API"] --> E["MCP servers"]
  F["GitHub ingest"] --> G["GitHub API"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Introduce Protocols/TypedDicts for tool/message shapes
  • ➕ Preserves type safety end-to-end instead of Any/casts
  • ➕ Makes contract with OpenAI/MCP tooling clearer and self-documenting
  • ➖ More upfront work; may require mirroring third-party types that change
  • ➖ Can be hard to keep aligned with dynamic tool schemas used by providers
2. Adjust type-checker configuration for third-party boundaries
  • ➕ Less code churn if errors are primarily from external library typing gaps
  • ➕ Keeps runtime code untouched
  • ➖ Suppresses potentially real type issues in app code
  • type: ignore / config exceptions can accumulate and hide regressions

Recommendation: The PR’s approach (replace type: ignore with explicit casts/Any at known dynamic boundaries) is a pragmatic fix to unblock ty while keeping behavior unchanged. If typing coverage is a priority long-term, consider incrementally replacing Any at tool/message boundaries with small Protocol/TypedDict definitions.

Files changed (6) +60 / -16

Refactor (4) +16 / -16
ingest.pyReplace duck-typed GitHub content ignores with explicit casts +3/-3

Replace duck-typed GitHub content ignores with explicit casts

• Adds 'typing.cast' and uses it when appending GitHub content objects that are duck-typed to 'ContentFile'. Removes '# type: ignore[arg-type]' while preserving existing runtime behavior.

src/ingest.py

methods.pyLoosen MCP tool dict typing to satisfy Responses API signatures +7/-7

Loosen MCP tool dict typing to satisfy Responses API signatures

• Annotates MCP tool specs as 'Any' and removes 'type: ignore' on 'tools=[...]' calls to the Responses API. This reflects the dynamic tool schema expected by the client library.

src/methods.py

workflow.pyCast message/tool/state types at LangGraph/OpenAI boundaries +4/-4

Cast message/tool/state types at LangGraph/OpenAI boundaries

• Casts 'messages', 'tools', and 'StateGraph' generic arguments to 'Any' to match third-party typing expectations. Removes invalid-argument 'type: ignore' markers without changing workflow execution.

src/workflow.py

streamlit_app.pyRelax workflow parameter typing and remove invoke ignore +2/-2

Relax workflow parameter typing and remove invoke ignore

• Changes the async workflow runner parameter type from 'Workflow' to 'Any' and removes the ignored 'invoke' attribute access. Keeps the invocation contract the same while satisfying the type checker.

streamlit_app.py

Documentation (1) +6 / -0
README.mdDocument swapping pyproject files for server vs app runs +6/-0

Document swapping pyproject files for server vs app runs

• Updates run instructions to copy the appropriate pyproject variant before 'uv sync' when running Llama Stack vs the Streamlit app. This clarifies dependency differences between the server/container setup and the local application environment.

README.md

Other (1) +38 / -0
pyproject.orig.tomlAdd application-focused pyproject baseline +38/-0

Add application-focused pyproject baseline

• Introduces a new pyproject file capturing the Streamlit app’s dependency set and tooling configuration (ruff, dev deps). Intended to be copied into 'pyproject.toml' when running the application locally.

pyproject.orig.toml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. README dirties dependency files 🐞 Bug ⚙ Maintainability
Description
README.md now instructs users to overwrite the tracked pyproject.toml in-place (twice) and run `uv
sync`, which can also update the tracked uv.lock and leave the repo in a dirty state. If those
changes are accidentally committed, the app container build can fail because it uses `uv sync
--frozen` and requires uv.lock to match pyproject.toml exactly.
Code

README.md[R74-77]

+cp pyproject.llama-stack.toml pyproject.toml
+uv sync
+source .venv/bin/activate
uv run llama stack run run.yaml
Evidence
README explicitly instructs copying alternate pyproject files over the tracked pyproject.toml and
running uv sync. The app Containerfile treats pyproject.toml and uv.lock as build inputs and
uses uv sync --frozen, so committing a mismatched pair after following the README steps can break
the image build.

README.md[69-78]
README.md[120-127]
Containerfile[22-27]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
README instructions overwrite `pyproject.toml` in-place (switching between `pyproject.llama-stack.toml` and `pyproject.orig.toml`) and rerun `uv sync`. This commonly leaves modified tracked files (`pyproject.toml`, and potentially `uv.lock`) in the working tree, increasing the chance of accidental commits; those commits can break the container build because it relies on `uv sync --frozen`.

## Issue Context
- The app image build copies `pyproject.toml` + `uv.lock` and runs `uv sync --frozen`.
- The README now suggests swapping `pyproject.toml` for different dependency sets.

## Fix Focus Areas
- README.md[71-78]
- README.md[120-127]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread README.md
Comment on lines +74 to 77
cp pyproject.llama-stack.toml pyproject.toml
uv sync
source .venv/bin/activate
uv run llama stack run run.yaml

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Readme dirties dependency files 🐞 Bug ⚙ Maintainability

README.md now instructs users to overwrite the tracked pyproject.toml in-place (twice) and run `uv
sync`, which can also update the tracked uv.lock and leave the repo in a dirty state. If those
changes are accidentally committed, the app container build can fail because it uses `uv sync
--frozen` and requires uv.lock to match pyproject.toml exactly.
Agent Prompt
## Issue description
README instructions overwrite `pyproject.toml` in-place (switching between `pyproject.llama-stack.toml` and `pyproject.orig.toml`) and rerun `uv sync`. This commonly leaves modified tracked files (`pyproject.toml`, and potentially `uv.lock`) in the working tree, increasing the chance of accidental commits; those commits can break the container build because it relies on `uv sync --frozen`.

## Issue Context
- The app image build copies `pyproject.toml` + `uv.lock` and runs `uv sync --frozen`.
- The README now suggests swapping `pyproject.toml` for different dependency sets.

## Fix Focus Areas
- README.md[71-78]
- README.md[120-127]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread README.md
## Run the application Code

```bash
cp pyproject.orig.toml pyproject.toml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHould we delete the pyproject.toml from main? Because it is still present in the repo.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I wasn't sure @maysunfaisal .... I suspect it is better to have one in place just in case, so that the compiling etc. of the python project is viable out of the box

I also considered creating a "run llama stack" sub directory, and just move that pyproject.toml there, and then change the directions to cd into that directory, do the other stuff, etc.

WDYT ... maybe I should pivot to the subdirectory ? Any other ideas on how to better manager this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants