feat: portfolio sprint 2 — principal-grade artifacts and supply-chain hardening#41
Conversation
- Delete raw AI session transcripts and binary planning files (convo.txt, chatgpt covo.txt, Repo Name Suggestions.docx/.pdf) from dev/planning/ - Archive dev/planning/ → docs/planning/ with historical-context README - Relocate .full-review/ → docs/review-process/; gitignore state.json - Add PORTFOLIO.md: stack context, where to start, design decisions, CI facts - Add README "For Reviewers" section linking arch.md, anti-scope, examples - Add docs/adr/README.md stub (Sprint 2 will populate with 3 ADRs) - Add dev/PORTFOLIO_BACKLOG.md: 16-item sprint-planned portfolio backlog - Strip 57 redundant @pytest.mark.asyncio decorators (asyncio_mode=auto) 387 tests pass, 85% coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… hardening ADRs (PB-02): - docs/adr/001-structural-protocols.md: PEP 544 protocols over ABC - docs/adr/002-flat-layout.md: flat layout over src/ wrapper - docs/adr/003-single-provider.md: single OpenAI-compatible Provider over adapter matrix - docs/adr/README.md: replace placeholder stub with proper index table Supply-chain security: - requirements.lock: pin all 30 dev deps via uv pip compile (PB-08) - .github/workflows/ci.yml: add pip-audit step to security job (PB-08) - .github/workflows/codeql.yml: new CodeQL SAST workflow for Python (PB-09) - sbom.json: CycloneDX 1.6 SBOM, 372 components (PB-10) - .github/workflows/publish.yml: regenerate SBOM on each release (PB-10) Type coverage (PB-15): - pyproject.toml: remove examples/ from mypy --strict exclusion list - examples/react_tool_use.py: fix 5 mypy errors (split _SAFE_OPS into typed Callable dicts, annotate provider params as ToolCallingProvider); fix 3 ruff errors (UP035 Callable import, TC006 cast quote) Note: PB-11 (OIDC trusted publishing) was already implemented in publish.yml. Sprint 2 Definition of Done: - 387 tests pass, 85% coverage - mypy --strict clean across 26 source files (executionkit/ + examples/) - ruff clean across all files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3335a72c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pip install cyclonedx-bom | ||
| cyclonedx-py environment --of JSON --output-file sbom.json |
There was a problem hiding this comment.
In the release workflow this runs cyclonedx-py environment immediately after installing only build and cyclonedx-bom, before executionkit or its locked dependencies are installed. I checked the CycloneDX CLI docs: the environment subcommand builds from the actually installed/current Python environment, so release SBOM artifacts will inventory the SBOM/build tooling environment rather than the distribution being published; the committed sbom.json shows the same failure mode by listing unrelated packages such as Authlib that are absent from pyproject.toml and requirements.lock. Generate from a clean project environment or the lock/requirements file with the project metadata instead.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request focuses on enhancing the project's documentation and architectural clarity by adding a portfolio orientation guide, a detailed backlog for future improvements, and several Architecture Decision Records (ADRs) regarding structural protocols and package layout. It also includes technical refinements such as adding a lockfile, restructuring historical planning documents, and enabling type checking for the examples directory. Review feedback suggests correcting a technical inaccuracy in the protocols ADR, improving type annotations in the examples to avoid brute-force casting, and further expanding type-checking coverage to include the test suite.
| ### Positive Consequences | ||
|
|
||
| * Any object with the right method signatures satisfies `LLMProvider` — including `MockProvider`, custom wrappers, and third-party clients. | ||
| * `@runtime_checkable` enables `isinstance(provider, LLMProvider)` checks inside `_TrackedProvider.supports_tools` without requiring inheritance. |
There was a problem hiding this comment.
This sentence contains a technical inaccuracy regarding the implementation. _TrackedProvider.supports_tools (in executionkit/patterns/base.py) is a hardcoded Literal[True] field; it does not perform an isinstance check. The ADR should accurately reflect that @runtime_checkable enables external components (like react_loop) to verify the provider's capabilities without requiring explicit inheritance.
| provider = cast( | ||
| "ToolCallingProvider", | ||
| Provider( | ||
| base_url="https://api.openai.com/v1", | ||
| api_key=os.environ["OPENAI_API_KEY"], | ||
| model="gpt-4o-mini", | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Using cast with a string literal for a type that is already imported is suboptimal. For a "principal-grade" artifact, it is preferable to use a type annotation on the variable. This allows the type checker to verify the implementation structurally against the protocol without the brute-force override of a cast.
provider: ToolCallingProvider = Provider(
base_url="https://api.openai.com/v1",
api_key=os.environ["OPENAI_API_KEY"],
model="gpt-4o-mini",
)| python_version = "3.11" | ||
| strict = true | ||
| exclude = ["tests/", "examples/"] | ||
| exclude = ["tests/"] |
There was a problem hiding this comment.
While removing examples/ from the mypy exclusion is a significant improvement, tests/ remains excluded. As noted in the project's own architecture review (Finding BP-H2), this allows type errors in test fixtures and shared helpers to go unnoticed. To achieve the stated goal of "principal-grade artifacts," the test suite should also be subject to type checking, potentially with a dedicated override section for more relaxed rules if necessary.
| exclude = ["tests/"] | |
| exclude = [] |
3 participants
Summary
requirements.lock(30 pinned packages viauv pip compile) +pip-auditstep in the CI security job; no vulnerabilities found.github/workflows/codeql.ymlrunning CodeQL SAST withsecurity-extendedqueries on push/PR/weekly schedulesbom.json(CycloneDX 1.6, 372 components) committed to root;publish.ymlregenerates it on each release tagexamples/frommypy --strictexclusion; fixed 5 type errors and 3 ruff issues inreact_tool_use.pyWhy
Sprint 2 goal from
dev/PORTFOLIO_BACKLOG.md: "Principal-grade artifacts are now discoverable and supply-chain posture is documented." A director-level reviewer landing on this repo can now find:Test plan
pytest tests/ -m "not integration"— 387 passed, 85% coverage (threshold: 80%)mypy --strict executionkit/ examples/— 0 errors across 26 source filesruff check executionkit/ tests/ examples/— all checks passedpip-audit --requirement requirements.lock— no known vulnerabilitiessbom.jsonvalid JSON, CycloneDX 1.6 format, 372 componentsFiles changed
docs/adr/001-structural-protocols.mddocs/adr/002-flat-layout.mddocs/adr/003-single-provider.mddocs/adr/README.mdrequirements.locksbom.json.github/workflows/codeql.yml.github/workflows/ci.yml.github/workflows/publish.ymlpyproject.tomlexamples/from mypy excludeexamples/react_tool_use.py🤖 Generated with Claude Code