Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new internal documentation hierarchy under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
harumiWeb
changed the title
There was a problem hiding this comment.
Pull request overview
This PR introduces an AI agent–oriented document retention policy and establishes/expands the internal documentation structure (dev-docs/) to better separate public contracts from internal specs and ADRs, updating task-tracking docs accordingly.
Changes:
- Added a document retention/promotion policy to
AGENTS.md, and recorded the policy intasks/feature_spec.mdandtasks/todo.md. - Added internal documentation structure under
dev-docs/(ADR templates + initial ADRs, internal specs, architecture notes, agent guides, testing requirements). - Updated
uv.lockto match the current editable package version (exstruct0.6.1).
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Updates locked editable package version for exstruct to 0.6.1. |
| tasks/todo.md | Records the new retention-policy task and review notes. |
| tasks/feature_spec.md | Captures the retention policy as a feature-spec “contract”. |
| dev-docs/testing/test-requirements.md | Adds a test requirements specification document (needs corrections noted in comments). |
| dev-docs/testing/README.md | Introduces the testing docs directory purpose. |
| dev-docs/specs/patch/model-migration-notes.md | Documents migration notes for Patch models (Phase 2). |
| dev-docs/specs/patch/legacy-dependency-inventory.md | Inventories removed legacy dependencies and replacements (Phase 2). |
| dev-docs/specs/excel-extraction.md | Adds internal spec for Excel extraction behavior (needs corrections noted in comments). |
| dev-docs/specs/data-model.md | Adds internal canonical data model spec. |
| dev-docs/specs/README.md | Adds guidance on how to read internal specs. |
| dev-docs/architecture/pipeline.md | Documents pipeline architecture and responsibilities. |
| dev-docs/architecture/overview.md | Documents overall architecture/module responsibilities. |
| dev-docs/architecture/contributor-guide.md | Provides internal contributor guide (contains incorrect paths noted in comments). |
| dev-docs/architecture/README.md | Adds index/overview for architecture docs directory. |
| dev-docs/agents/roadmap.md | Adds agent-facing product roadmap document. |
| dev-docs/agents/overview.md | Adds agent-facing system overview (needs mode list correction noted in comments). |
| dev-docs/agents/contributing.md | Adds agent-specific contribution guidelines (contains module responsibility mismatch noted in comments). |
| dev-docs/agents/coding-guidelines.md | Adds detailed coding guidelines for agents and humans. |
| dev-docs/agents/README.md | Adds entrypoint README for agent docs. |
| dev-docs/adr/template.md | Adds ADR template. |
| dev-docs/adr/README.md | Adds ADR directory README/usage guidelines. |
| dev-docs/adr/ADR-0001-extraction-mode-boundaries.md | Adds ADR documenting extraction mode boundaries. |
| dev-docs/adr/ADR-0002-rich-backend-fallback-policy.md | Adds ADR documenting rich-backend fallback policy. |
| dev-docs/adr/ADR-0003-output-serialization-omission-policy.md | Adds ADR documenting serialization omission defaults. |
| dev-docs/adr/ADR-0004-patch-backend-selection-policy.md | Adds ADR documenting patch backend selection policy. |
| dev-docs/adr/ADR-0005-path-policy-safety-boundary.md | Adds ADR documenting PathPolicy safety boundary. |
| dev-docs/README.md | Adds top-level internal docs README (roles and reading order). |
| AGENTS.md | Adds the “document retention policy” section to guide agent behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
dev-docs/agents/contributing.md (1)
33-36: 配置先パスはsrc/exstruct/...まで明記した方が安全です。Line 33-36 の
core/,models/,io/,cli/は解釈が分かれるため、誤配置防止のためにリポジトリ相対で明示するのを推奨します。✍️ Suggested doc tweak
-- 新しい抽出機能, 意味解析アルゴリズム → core/ -- 新しいデータ構造 → models/ -- 出力形式追加 → io/ -- CLI 機能 → cli/ +- 新しい抽出機能, 意味解析アルゴリズム → `src/exstruct/core/` +- 新しいデータ構造 → `src/exstruct/models/` +- 出力形式追加 → `src/exstruct/io/` +- CLI 機能 → `src/exstruct/cli/`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-docs/agents/contributing.md` around lines 33 - 36, Replace ambiguous directory mentions with repository-relative explicit paths to avoid misplacement: update the entries `core/`, `models/`, `io/`, `cli/` to `src/exstruct/core/`, `src/exstruct/models/`, `src/exstruct/io/`, `src/exstruct/cli/` in the contributing doc (the block containing the four list items) so readers have a clear, unambiguous target location.dev-docs/agents/roadmap.md (1)
11-51: Consider clarifying version numbering strategy.The roadmap shows incremental features across versions with some unusual version numbers (v0.2.80, v0.2.90). If these represent historical releases, consider adding a note indicating which versions are already released vs. planned. This would help readers understand the current state vs. future plans.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-docs/agents/roadmap.md` around lines 11 - 51, The roadmap has ambiguous version numbers (e.g., headings "## v0.2.80", "## v0.2.90") that could be historical or planned; update dev-docs/agents/roadmap.md to add a short clarifying note at the top (above "## v0.2.0") describing the versioning strategy and mark which headings are released versus planned (for example annotate or group headings under "Released" and "Planned" or append "(released)" / "(planned)" to headings like "## v0.2.80" and "## v0.3.0"); keep the note concise and ensure it references the existing version headings so readers can immediately tell current state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev-docs/agents/overview.md`:
- Around line 9-10: モード一覧の記述 "mode（light/standard/verbose）"
が現行の4モード契約と合っていないため、モードリストに "libreoffice" を追加して4つ（light, standard, verbose,
libreoffice）に揃えてください；該当箇所はドキュメント内のモード説明行（現在 "mode（light/standard/verbose）"
と表記されている箇所）を編集し、文中でモード数を数える説明やサンプルがあれば同様に "libreoffice" を反映させて一貫性を保ってください。
In `@dev-docs/architecture/overview.md`:
- Around line 3-5: 概要文の説明が現行 backend を取りこぼしているので、ExStruct のアーキテクチャ要約（現在の
openpyxl と Excel COM（xlwings）を説明している箇所）に LibreOffice 経路（libreoffice
backend）を明記し、パイプライン内での役割分担とどの入力に対して LibreOffice を使うかを一文で追加してください（参照箇所: ExStruct,
openpyxl, Excel COM（xlwings）, libreoffice）。
In `@dev-docs/specs/excel-extraction.md`:
- Around line 18-22: The spec is missing the "libreoffice" extraction mode
declared in the ExtractionMode type; update the modes list to include a fourth
bullet for "libreoffice" and describe its behavior (how it differs from
light/standard/verbose and when it is used), e.g., that it invokes
LibreOffice-based extraction to capture text from embedded shapes/objects and
preserves layout/formatting not handled by COM, and document any size/chart
handling or fallbacks consistent with the implementation of the extraction
pipeline; reference the ExtractionMode type and ensure the new bullet matches
the exact literal "libreoffice".
In `@dev-docs/specs/patch/model-migration-notes.md`:
- Line 21: 現在の記述「先行して別モデル化」は誤解を招くので、patch/models.py に既に存在する PatchDiffItem /
PatchErrorDetail / FormulaIssue を先に定義したことを問題視するのではなく、PatchDiffItem 系と
PatchErrorDetail/FormulaIssue 系の2つの独立したクラス階層（独立したバリデーションルール）が存在するため、PatchResult
の構築時に相互検証が失敗しやすい、という原因を明確に説明するよう文言を差し替えてください（参照シンボル: PatchDiffItem,
PatchErrorDetail, FormulaIssue, PatchResult）。
- Around line 13-17: The migration steps currently describe moving model
definitions from legacy_runner.py to patch/models.py, but since patch/models.py
already contains the canonical models and duplicate definitions remain in
internal.py, update the steps to remove duplicate definitions in internal.py
(and any other files) rather than moving them; specifically, ensure references
to legacy_runner.py, patch/models.py, internal.py, and patch_runner.py instruct
to: delete duplicate model and PatchOp-related validation code from internal.py,
consolidate type annotations to import from patch.models, update
legacy_runner.py to only import from patch.models (or be removed ultimately),
and adjust tests under tests/mcp/ to point at patch.models and patch_runner.py
as the single source of truth.
- Around line 5-9: The migration notes incorrectly claim model definitions live
only in legacy_runner; in reality there are duplicate hierarchies
(PatchDiffItem, PatchErrorDetail, FormulaIssue, PatchResult) in both
patch/models.py and patch/internal.py (legacy_runner). Fix by removing the
duplicate class definitions in the legacy/internal module and importing the
canonical models from patch.models everywhere (replace local class defs in
internal.py with from patch.models import PatchDiffItem, PatchErrorDetail,
FormulaIssue, PatchResult), or if you must keep legacy instances, normalize
before validation by converting legacy instances to the canonical types (e.g.,
construct PatchResult and PatchDiffItem via PatchResult.parse_obj(...) or
PatchDiffItem.parse_obj(...) or dict serialization) so Pydantic validation
receives instances of the same model classes.
---
Nitpick comments:
In `@dev-docs/agents/contributing.md`:
- Around line 33-36: Replace ambiguous directory mentions with
repository-relative explicit paths to avoid misplacement: update the entries
`core/`, `models/`, `io/`, `cli/` to `src/exstruct/core/`,
`src/exstruct/models/`, `src/exstruct/io/`, `src/exstruct/cli/` in the
contributing doc (the block containing the four list items) so readers have a
clear, unambiguous target location.
In `@dev-docs/agents/roadmap.md`:
- Around line 11-51: The roadmap has ambiguous version numbers (e.g., headings
"## v0.2.80", "## v0.2.90") that could be historical or planned; update
dev-docs/agents/roadmap.md to add a short clarifying note at the top (above "##
v0.2.0") describing the versioning strategy and mark which headings are released
versus planned (for example annotate or group headings under "Released" and
"Planned" or append "(released)" / "(planned)" to headings like "## v0.2.80" and
"## v0.3.0"); keep the note concise and ensure it references the existing
version headings so readers can immediately tell current state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ef4aac7f-cd4b-47cc-9600-559ce5586797
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
AGENTS.mddev-docs/README.mddev-docs/adr/ADR-0001-extraction-mode-boundaries.mddev-docs/adr/ADR-0002-rich-backend-fallback-policy.mddev-docs/adr/ADR-0003-output-serialization-omission-policy.mddev-docs/adr/ADR-0004-patch-backend-selection-policy.mddev-docs/adr/ADR-0005-path-policy-safety-boundary.mddev-docs/adr/README.mddev-docs/adr/template.mddev-docs/agents/README.mddev-docs/agents/coding-guidelines.mddev-docs/agents/contributing.mddev-docs/agents/overview.mddev-docs/agents/roadmap.mddev-docs/architecture/README.mddev-docs/architecture/contributor-guide.mddev-docs/architecture/overview.mddev-docs/architecture/pipeline.mddev-docs/specs/README.mddev-docs/specs/data-model.mddev-docs/specs/excel-extraction.mddev-docs/specs/patch/legacy-dependency-inventory.mddev-docs/specs/patch/model-migration-notes.mddev-docs/testing/README.mddev-docs/testing/test-requirements.mdtasks/feature_spec.mdtasks/todo.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
dev-docs/architecture/overview.md (1)
10-20: Unify repository path notation in this doc.Line 10 uses
exstruct/...while Line 78 switches tosrc/exstruct/.... Pick one convention to avoid navigation mistakes in contributor workflows.✍️ Suggested doc diff
-exstruct/ +src/exstruct/ core/ @@ -### mcp/patch（Patch 実装） +### mcp/patch（Patch 実装） @@ -Patch 系は `src/exstruct/mcp/patch/` に責務分離して実装する。 +Patch 系は `src/exstruct/mcp/patch/` に責務分離して実装する。Also applies to: 78-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-docs/architecture/overview.md` around lines 10 - 20, The document uses two different repository path notations ("exstruct/" and "src/exstruct/"); unify them by choosing one convention (prefer project root relative, e.g., "src/exstruct/" or repository-root "exstruct/") and update every occurrence to that chosen form (search for "exstruct/" and "src/exstruct/" and replace consistently), including the directory tree block (core, pipeline.py, integrate.py, modeling.py, workbook.py, backends/base.py, backends/openpyxl_backend.py, backends/com_backend.py, backends/libreoffice_backend.py) so all references match the same path style throughout the doc.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev-docs/testing/test-requirements.md`:
- Line 118: Update the test requirement labeled [MOD-02] which currently
references "DATA_MODEL.md" to use the canonical path
"dev-docs/specs/data-model.md"; locate the string "DATA_MODEL.md" in the
requirement text for [MOD-02] and replace it with "dev-docs/specs/data-model.md"
so the requirement points to the correct canonical data-model reference.
- Line 210: Update the Windows smoke contract MODE-05d so
EXSTRUCT_LIBREOFFICE_PATH is specified to prefer "soffice.com" with a fallback
to "soffice.exe" instead of hard-coding "soffice.exe"; edit the test requirement
text that mentions `EXSTRUCT_LIBREOFFICE_PATH` and `soffice.exe` to describe the
preferred selection order (soffice.com first, then soffice.exe) and ensure the
pytest.mark.libreoffice smoke job instructions reflect this stabilization
guidance.
---
Nitpick comments:
In `@dev-docs/architecture/overview.md`:
- Around line 10-20: The document uses two different repository path notations
("exstruct/" and "src/exstruct/"); unify them by choosing one convention (prefer
project root relative, e.g., "src/exstruct/" or repository-root "exstruct/") and
update every occurrence to that chosen form (search for "exstruct/" and
"src/exstruct/" and replace consistently), including the directory tree block
(core, pipeline.py, integrate.py, modeling.py, workbook.py, backends/base.py,
backends/openpyxl_backend.py, backends/com_backend.py,
backends/libreoffice_backend.py) so all references match the same path style
throughout the doc.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7495ba21-5ae1-40cd-bb92-49bb4f3eba48
📒 Files selected for processing (9)
dev-docs/agents/contributing.mddev-docs/agents/overview.mddev-docs/architecture/contributor-guide.mddev-docs/architecture/overview.mddev-docs/specs/excel-extraction.mddev-docs/specs/patch/model-migration-notes.mddev-docs/testing/test-requirements.mdtasks/feature_spec.mdtasks/todo.md
✅ Files skipped from review due to trivial changes (3)
- dev-docs/specs/excel-extraction.md
- dev-docs/specs/patch/model-migration-notes.md
- dev-docs/agents/overview.md
🚧 Files skipped from review as they are similar to previous changes (2)
- dev-docs/architecture/contributor-guide.md
- dev-docs/agents/contributing.md
Owner
Author
|
補足: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Closes #89
Summary by CodeRabbit