Bug/148 hwp parsing error - #341
Conversation
|
Warning Review limit reached
Next review available in: 28 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe attachment processor now supports compact Markdown table serialization by default. The setting propagates through recursive and hybrid processing, supports runtime overrides, and falls back to the existing exporter on errors. Unit and HWP regression tests cover output and table preservation. ChangesCompact Markdown table serialization
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@genon/preprocessor/facade/attachment_processor.py`:
- Line 1017: Parse and validate the compact_tables runtime override before the
selection at attachment_processor.py:1017-1017, rejecting invalid values and
interpreting "false" as False rather than applying bool() directly. Pass the
resulting Boolean through the DOCX and HWP recursive splitter calls at
attachment_processor.py:1455-1455 and attachment_processor.py:1661-1661. Add
unit coverage at
genon/preprocessor/tests/unit/test_attachment_compact_tables_unit.py:178-183 for
"false" and invalid runtime values.
In `@genon/preprocessor/tests/regression/test_hwp_table_structure_regression.py`:
- Line 61: Update the whitespace normalization expressions in the affected test
helper to replace the raw non-breaking-space literals with the Unicode escape
"\u00a0", while preserving the existing replacement and stripping behavior and
resolving RUF001 warnings.
- Around line 463-468: Update the environment override logic around
GENOS_LAYOUT_ENDPOINT and GENOS_LAYOUT_API_KEY so an overridden endpoint is only
accepted when an API key is explicitly provided via GENOS_LAYOUT_API_KEY;
otherwise reject or fail the configuration instead of retaining the YAML
api_key. Preserve the existing behavior when no endpoint override is supplied.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e7092ddd-6fe4-484c-8e56-93357deb0330
📒 Files selected for processing (8)
genon/.gitignoregenon/preprocessor/facade/attachment_processor.pygenon/preprocessor/facade/gitbook_doc/attachment_processor.mdgenon/preprocessor/resource/attachment_processor_config.yamlgenon/preprocessor/resource_dev/attachment_processor_config.yamlgenon/preprocessor/sample_files/hwp_sample_table.hwpgenon/preprocessor/tests/regression/test_hwp_table_structure_regression.pygenon/preprocessor/tests/unit/test_attachment_compact_tables_unit.py
| endpoint = os.environ.get("GENOS_LAYOUT_ENDPOINT") | ||
| if endpoint: | ||
| genos_layout["endpoint"] = endpoint | ||
| api_key = os.environ.get("GENOS_LAYOUT_API_KEY") | ||
| if api_key is not None: | ||
| genos_layout["api_key"] = api_key |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not send the configured API key to an overridden endpoint.
If GENOS_LAYOUT_ENDPOINT is set and GENOS_LAYOUT_API_KEY is absent, this code retains the API key from resource_dev/intelligent_processor_config.yaml. It can send that key to the environment-supplied endpoint. Require an API key whenever the endpoint is overridden.
Proposed fix
endpoint = os.environ.get("GENOS_LAYOUT_ENDPOINT")
- if endpoint:
- genos_layout["endpoint"] = endpoint
api_key = os.environ.get("GENOS_LAYOUT_API_KEY")
- if api_key is not None:
+ if endpoint:
+ if api_key is None:
+ raise RuntimeError(
+ "GENOS_LAYOUT_API_KEY is required when GENOS_LAYOUT_ENDPOINT is set"
+ )
+ genos_layout["endpoint"] = endpoint
genos_layout["api_key"] = api_key
+ elif api_key is not None:
+ genos_layout["api_key"] = api_key📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| endpoint = os.environ.get("GENOS_LAYOUT_ENDPOINT") | |
| if endpoint: | |
| genos_layout["endpoint"] = endpoint | |
| api_key = os.environ.get("GENOS_LAYOUT_API_KEY") | |
| if api_key is not None: | |
| genos_layout["api_key"] = api_key | |
| endpoint = os.environ.get("GENOS_LAYOUT_ENDPOINT") | |
| api_key = os.environ.get("GENOS_LAYOUT_API_KEY") | |
| if endpoint: | |
| if api_key is None: | |
| raise RuntimeError( | |
| "GENOS_LAYOUT_API_KEY is required when GENOS_LAYOUT_ENDPOINT is set" | |
| ) | |
| genos_layout["endpoint"] = endpoint | |
| genos_layout["api_key"] = api_key | |
| elif api_key is not None: | |
| genos_layout["api_key"] = api_key |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@genon/preprocessor/tests/regression/test_hwp_table_structure_regression.py`
around lines 463 - 468, Update the environment override logic around
GENOS_LAYOUT_ENDPOINT and GENOS_LAYOUT_API_KEY so an overridden endpoint is only
accepted when an API key is explicitly provided via GENOS_LAYOUT_API_KEY;
otherwise reject or fail the configuration instead of retaining the YAML
api_key. Preserve the existing behavior when no endpoint override is supplied.
2 participants
fix(#148): HWP 표 구조 회귀 테스트 추가 + 첨부용 markdown 표 compact 출력
개요
배경 — 보고된 두 가지 증상
[별표 제3호] 숙박급지표에서 말레이시아·브루나이·태국,필리핀 이 실제로는 병지인데AI 가 을지라고 오답했다.
"법인카드 사용내역이 청크에 없다"가 이 증상이다.
주요 변경
1) HWP 표 구조 회귀 테스트 신설 (핵심)
tests/regression/test_hwp_table_structure_regression.py— 테스트 9개.두 전처리기는 HWP 처리 방식과 표 출력 형식이 달라 각각 검증한다.
TestAttachmentHwpTableStructure(4개) — 표 청크 분할 여부, 별표3/별표4 구조, 표 개수 가드TestIntelligentHwpTableStructure(4개) — HTML 직렬화, 별표3/별표4 구조, 별표4 행 레이블test_rhwp_pdf_페이지_지오메트리(1개) — 변환 단계 분리 검증markdown/HTML 을 모두 "행 × 셀" 2차원 리스트로 정규화해 assert 로직을 한 벌만 두고
두 경로가 공유한다. golden 스냅샷이 아니라 의미 기반 assert 라 baseline 파일이 없다.
2) 첨부용 markdown 표 compact 출력
output.compact_tables(기본true) 신설. markdown 표의 컬럼 정렬 패딩을 제거해 대형 표의청크 크기를 줄인다. 표 내용은 동일하고 공백만 사라진다.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation