-
Notifications
You must be signed in to change notification settings - Fork 14
CodeCovの設定見直しと新アーキテクチャのドキュメント拡充 #28
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
Conversation
📝 WalkthroughWalkthroughUpdates CI to skip Excel COM tests on Windows, classifies coverage with Codecov flags, and adds architecture and contributor documentation plus README and docs edits; also adds dev tasks in pyproject and excludes architecture/contributors from the docs build. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/pytest.yml (1)
38-38: Consider removing unused SKIP_COM_TESTS environment variable.The
SKIP_COM_TESTS: "1"environment variable appears to be unused, as test selection is controlled by the pytest marker expression-m "not com and not render"on Line 41. If this environment variable isn't referenced elsewhere in the test suite, consider removing it to reduce potential confusion.🔎 Verification script to check if SKIP_COM_TESTS is referenced
#!/bin/bash # Search for references to SKIP_COM_TESTS in the codebase rg -n 'SKIP_COM_TESTS' --type py
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.github/workflows/pytest.ymlREADME.ja.mdREADME.mdcodecov.ymldocs/README.en.mddocs/README.ja.mddocs/agents/TEST_REQUIREMENTS.mddocs/architecture/pipeline.mddocs/contributors/architecture.mdmkdocs.ymlpyproject.toml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-27T14:32:54.706Z
Learnt from: CR
Repo: harumiWeb/exstruct PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T14:32:54.706Z
Learning: Applies to **/*.py : Generated code must pass Ruff linter checks (E, W, F, I, B, UP, N, C90 rules) with zero errors
Applied to files:
pyproject.toml
📚 Learning: 2025-12-27T14:32:54.706Z
Learnt from: CR
Repo: harumiWeb/exstruct PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T14:32:54.706Z
Learning: Applies to **/*.py : Do not depend on internal structures of external libraries (xlwings, pandas, numpy); normalize external entities to Pydantic models at the boundary layer
Applied to files:
docs/contributors/architecture.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (11)
README.md (1)
383-396: LGTM! Clean documentation additions.The new Architecture and Contributing sections provide clear entry points for developers to understand the codebase structure and contribution guidelines. The relative links to the detailed documentation pages are appropriate.
mkdocs.yml (1)
7-8: LGTM! Appropriate exclusion of internal documentation.Excluding the
architecture/andcontributors/directories from the public documentation build is the correct approach, as these contain internal implementation details and contributor-specific guidance that are not part of the user-facing API documentation.docs/agents/TEST_REQUIREMENTS.md (1)
244-260: LGTM! Clear documentation for local COM test execution.The section appropriately documents the CI limitation for Excel COM tests and provides clear instructions for local execution. The task commands (
task test-unit,task test-com) correctly reference the definitions added inpyproject.toml, and the codecov upload syntax with the-Fflag is correct for specifying coverage flags.pyproject.toml (1)
128-133: LGTM! Well-structured test and coverage tasks.The test separation into
test-unitandtest-comtargets is clear and aligns with the CI strategy. The codecov tasks correctly use the-Fflag to specify coverage flags that match thecodecov.ymlconfiguration. The documentation taskbuild-docsappropriately combines schema generation and documentation building.README.ja.md (1)
384-397: LGTM! Consistent documentation updates in Japanese README.The Architecture and Contributing sections mirror the English README appropriately, maintaining consistency across language versions. The links correctly point to the detailed documentation pages.
docs/contributors/architecture.md (1)
1-202: Excellent contributor guide with clear architectural principles.This comprehensive guide effectively documents the three-layer architecture (Pipeline, Backend, Modeling) and provides practical extension patterns. The design rules, testing guidelines, and anti-patterns section will help contributors avoid common mistakes and maintain architectural consistency.
The clear separation of concerns and the emphasis on localized failure points align well with the project's goal of stable structure extraction from fragile Excel sources.
codecov.yml (1)
1-18: LGTM! Appropriate codecov configuration for unit/COM test separation.The configuration correctly separates unit and COM coverage with distinct flags. The
carryforward: truesetting for thecomflag is appropriate since COM tests cannot run in CI and need to preserve coverage from local runs. The lenient thresholds (1%) align well with the project's incremental coverage improvement strategy..github/workflows/pytest.yml (1)
41-47: LGTM! Correct test exclusion and coverage flag.The pytest marker exclusion
-m "not com and not render"correctly restricts Windows CI tests to the unit test suite, and theflags: unitparameter properly classifies the coverage upload to match thecodecov.ymlconfiguration.docs/README.ja.md (1)
390-403: Architecture and Contributing sections are well-integrated.The new sections are properly positioned after the Documentation build section and logically flow from architecture overview to contributing guidance. The Japanese text is clear and consistent with the rest of the README. Links follow the established format for internal references.
Please verify that
docs/contributors/architecture.mdexists in this PR. Both the Japanese and English READMEs reference this file, and it must be present for the links to work correctly.docs/architecture/pipeline.md (1)
1-111: Architecture documentation is comprehensive and well-structured.The pipeline architecture guide clearly explains the three-layer separation of concerns, provides a helpful Mermaid sequence diagram, and addresses both current design and future extensibility. The Fallback Design section is particularly valuable for developers building batch/CI workflows. Markdown formatting and syntax are correct throughout.
docs/README.en.md (1)
32-34: Documentation additions are well-integrated and clearly written.The platform note, output modes detail, and enterprise section provide essential context for users across different environments. The Architecture and Contributing sections are logically placed and align well with the PR objective of establishing documentation infrastructure. Markdown formatting is consistent throughout.
Please verify that
docs/contributors/architecture.mdexists in this PR. The Contributing section at lines 394-399 links to this file, which must be present for users to access the contributor guidelines.Also applies to: 133-135, 368-371, 386-399
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.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
docs/agents/TEST_REQUIREMENTS.mdpyproject.toml
✅ Files skipped from review due to trivial changes (1)
- docs/agents/TEST_REQUIREMENTS.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-27T14:32:54.706Z
Learnt from: CR
Repo: harumiWeb/exstruct PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T14:32:54.706Z
Learning: Applies to **/*.py : Generated code must pass Ruff linter checks (E, W, F, I, B, UP, N, C90 rules) with zero errors
Applied to files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
pyproject.toml (2)
128-129: LGTM: Clean separation of unit and COM tests.The test tasks properly separate unit tests from COM-dependent tests using pytest marks. This aligns well with the CI workflow changes that skip COM tests on Windows.
27-27: Version 11.2.6 is current and has no known security vulnerabilities; migration is correct.This correctly addresses the previous review comment about replacing the deprecated
codecovpackage withcodecov-cli. Version 11.2.6 (released December 5, 2025) is the latest available on PyPI with no known CVEs or security advisories.
| codecov-unit = "codecov-cli upload-process -f coverage.xml -F unit -C %CODECOV_SHA% -t %CODECOV_TOKEN%" | ||
| codecov-com = "codecov-cli upload-process -f coverage.xml -F com -C %CODECOV_SHA% -t %CODECOV_TOKEN%" |
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.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the pyproject.toml file to verify the exact content
find . -name "pyproject.toml" -type f | head -1 | xargs cat | sed -n '120,140p'Repository: harumiWeb/exstruct
Length of output: 870
🏁 Script executed:
# Check for CI workflow files to understand which platforms run these tasks
find . -path "*/.github/workflows/*.yml" -o -path "*/.github/workflows/*.yaml" | head -5Repository: harumiWeb/exstruct
Length of output: 191
🏁 Script executed:
# Check taskipy documentation or README for cross-platform support
find . -type f \( -name "README*" -o -name "CONTRIBUTING*" \) | xargs grep -l "taskipy\|environment.*variable" 2>/dev/null | head -3Repository: harumiWeb/exstruct
Length of output: 44
🏁 Script executed:
cat ./.github/workflows/pytest.ymlRepository: harumiWeb/exstruct
Length of output: 1615
🏁 Script executed:
# Check if codecov-unit or codecov-com are referenced in any workflows
grep -r "codecov-unit\|codecov-com\|task run" ./.github/workflows/ --include="*.yml" --include="*.yaml"Repository: harumiWeb/exstruct
Length of output: 44
🏁 Script executed:
# Check taskipy installation and how it handles environment variables
grep -r "taskipy" . --include="pyproject.toml" --include="requirements*.txt" --include="*.md" --include="*.txt" | head -10Repository: harumiWeb/exstruct
Length of output: 139
🏁 Script executed:
# Search for any references to these tasks in the codebase
grep -r "codecov-unit\|codecov-com" . --exclude-dir=.git --exclude="*.pyc" --exclude-dir=__pycache__Repository: harumiWeb/exstruct
Length of output: 301
🏁 Script executed:
# Check if there are any development scripts or documentation that might reference these tasks
find . -type f \( -name "*.md" -o -name "*.rst" -o -name "Makefile" -o -name "*.sh" \) | xargs grep -l "task\|codecov" 2>/dev/nullRepository: harumiWeb/exstruct
Length of output: 96
🏁 Script executed:
# Check git history to see if these were recently added or if there's context
git log --oneline --all -20 -- pyproject.toml 2>/dev/null || echo "Git history not available"Repository: harumiWeb/exstruct
Length of output: 658
Remove or fix cross-platform environment variable syntax in codecov tasks.
The codecov-unit and codecov-com tasks use Windows-style environment variable syntax (%CODECOV_SHA%, %CODECOV_TOKEN%), which won't expand on Unix-like systems. While these tasks aren't currently invoked in CI (which uses the official codecov GitHub action), developers attempting to run task codecov-unit or task codecov-com locally on macOS or Linux will encounter failures. Either remove these unused tasks or update them to use cross-platform syntax (${CODECOV_SHA}, ${CODECOV_TOKEN}).
🤖 Prompt for AI Agents
In pyproject.toml around lines 130-131, the codecov-unit and codecov-com task
commands use Windows-style environment variable syntax (%CODECOV_SHA%,
%CODECOV_TOKEN%) which fails on Unix-like systems; either delete these unused
tasks or replace the Windows syntax with cross-platform POSIX-style
interpolation (e.g. ${CODECOV_SHA} and ${CODECOV_TOKEN}) so the variables expand
correctly on macOS/Linux when running the tasks locally.
Summary by CodeRabbit
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.