Skip to content

feat(py): middleware plugin + samples#5411

Open
huangjeff5 wants to merge 5 commits into
jh-mwfrom
jh-mw-plugin-v2
Open

feat(py): middleware plugin + samples#5411
huangjeff5 wants to merge 5 commits into
jh-mwfrom
jh-mw-plugin-v2

Conversation

@huangjeff5
Copy link
Copy Markdown
Contributor

@huangjeff5 huangjeff5 commented May 27, 2026

Summary

  • Python middleware plugin: Retry, Fallback, ToolApproval, Skills, Filesystem
  • Middleware sample and middleware-coding-agent sample

@github-actions github-actions Bot added docs Improvements or additions to documentation python Python config labels May 27, 2026
@huangjeff5 huangjeff5 changed the base branch from main to jh-mw May 27, 2026 00:05
Comment thread py/packages/genkit/src/genkit/_core/_reflection_v2.py Outdated
Comment thread py/packages/genkit/src/genkit/_core/_reflection.py Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 introduces a comprehensive middleware system for Genkit model calls, allowing users to inspect or modify requests and tool executions. It includes core abstractions, five concrete middleware implementations (Retry, Fallback, ToolApproval, Skills, and Filesystem), integration into the generation pipeline, and updates to reflection endpoints.

Feedback on the changes highlights three key areas for improvement:

  1. In _skills.py, the frontmatter parser _parse_skill_file should be updated to support CRLF line endings to prevent parsing failures on Windows systems.
  2. In utils.py (Google GenAI plugin), the logic for peeling off the outbound envelope from tool responses should be made more robust by verifying that the name key matches the actual function response name to avoid corrupting legitimate tool outputs.
  3. In _reflection.py, changing the return type annotation of values to Response may cause a NameError if Response is not imported; keeping it as JSONResponse is safer and more specific.
I am having trouble creating individual review comments. Click here to see my feedback.

py/plugins/middleware/src/genkit/plugins/middleware/_skills.py (76-92)

medium

The _parse_skill_file method currently checks content.startswith('---\n') and searches for \n---. This will fail to parse the frontmatter on Windows systems where files are saved with CRLF (\r\n) line endings, causing the middleware to fall back to using the directory name and losing the description. We should update this to support both LF and CRLF line endings.

    def _parse_skill_file(self, path: Path) -> tuple[str, str]:
        try:
            content = path.read_text(encoding='utf-8').lstrip('\\ufeff')
        except Exception:
            return '', ''
        if content.startswith('---\\r\\n'):
            start_idx = 5
            end_marker = '\\r\\n---'
        elif content.startswith('---\\n'):
            start_idx = 4
            end_marker = '\\n---'
        else:
            return '', ''
        end_idx = content.find(end_marker, start_idx)
        if end_idx == -1:
            return '', ''
        try:
            data = yaml.safe_load(content[start_idx:end_idx])
            if not isinstance(data, dict):
                return '', ''
            return data.get('name', ''), data.get('description', '')
        except Exception:
            return '', ''

py/plugins/google-genai/src/genkit/plugins/google_genai/models/utils.py (324-330)

medium

The current check isinstance(output, dict) and 'name' in output and 'content' in output is used to peel off the outbound envelope. However, if a tool legitimately returns a dictionary containing generic name and content keys, this check will incorrectly trigger and strip away the other keys, corrupting the tool output. We can make this check extremely robust by verifying that the name key in the dictionary matches the actual part.function_response.name.

        if part.function_response:
            # If the model echoes back the ``{name, content}`` envelope we
            # used on the outbound side, peel it off so the caller sees the
            # original tool output.
            output = part.function_response.response
            if isinstance(output, dict) and output.get('name') == part.function_response.name and 'content' in output:
                output = output['content']

py/packages/genkit/src/genkit/_core/_reflection.py (180)

medium

The return type annotation was changed from JSONResponse to Response. If Response is not imported in this file, this will raise a NameError at import time. Since both return paths in this function return a JSONResponse, it is safer and more specific to keep the return type annotation as JSONResponse.

    async def values(req: Request) -> JSONResponse:

@huangjeff5 huangjeff5 requested a review from pavelgj May 27, 2026 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config docs Improvements or additions to documentation python Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant