feat: JSONL 추출을 위한 export_jsonl.py 도구 추가 - #781
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Review limit reachedNext included review available in 43 seconds. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| record = { | ||
| "document_id": document_id, | ||
| "page_number": page_number, | ||
| "article_id": article.get("article_id", "Unknown Article ID"), | ||
| "headline": article.get("headline", ""), | ||
| "body_blocks": article.get("body_blocks", []), | ||
| } |
There was a problem hiding this comment.
🟡 기사의 이미지와 주석이 누락됨
유효한 기사도 record가 이미지, 캡션, 각주, 경계 상자를 제외한 다섯 필드만 내보냅니다. JSONL 데이터셋에서 해당 정보가 사라집니다.
| record = { | |
| "document_id": document_id, | |
| "page_number": page_number, | |
| "article_id": article.get("article_id", "Unknown Article ID"), | |
| "headline": article.get("headline", ""), | |
| "body_blocks": article.get("body_blocks", []), | |
| } | |
| record = { | |
| **article, | |
| "document_id": document_id, | |
| "page_number": page_number, | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| "not_a_dict_page", | ||
| { | ||
| "page_number": 2, | ||
| "articles": [ | ||
| "not_a_dict_article", |
There was a problem hiding this comment.
🔍 성공 테스트가 정식 스키마를 위반함
VALID_JSON_DATA는 문자열 페이지와 기사를 포함합니다. 실제 ParseResponse fixture로 전체 기사 필드 보존을 검증해야 합니다.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
added
enhancement
ChatGPT Codex Connector
![cwl-noema-review[bot]](https://avatars.githubusercontent.com/u/295022177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Noema LLM review
export_jsonl.py silently drops every article field except document_id, page_number, article_id, headline, and body_blocks. For a tool whose stated purpose is article-level dataset extraction, this is a confirmed data-loss bug (also flagged in the open review thread at tools/export_jsonl.py:39). The test fixture and success assertions only cover the five hardcoded fields, so the suite cannot detect the loss. The uv.lock changes (pypdf 6.15.0→6.16.2 within the declared >=6.15.0,<7.0 range; exceptiongroup marker) are benign and consistent with the changelog. Blocking fix: preserve all article fields (e.g., **article) and add a test using a realistic article containing images, captions, footnotes, and bounding boxes.
Reviewed changed lines
tools/export_jsonl.py:33 (RIGHT): The record dict is hardcoded to exactly five keys (document_id, page_number, article_id, headline, body_blocks). Any other keys present in a valid article (images, captions, footnotes, bounding_boxes, etc.) are silently dropped, contradicting the tool's purpose of exporting article data for dataset processing. Confirmed data-loss bug.tools/export_jsonl.py:39 (RIGHT): Closing brace of the record dict confirms the fixed five-key field set. The prior review thread's suggestion to use **article to preserve all fields is not implemented.tests/test_tools_export_jsonl.py:10 (RIGHT): VALID_JSON_DATA contains only the five hardcoded fields plus string page/article entries that are skipped. It does not reflect the real ParseResponse schema and cannot exercise images, captions, footnotes, or bounding boxes, so the test cannot catch the field-loss regression.tests/test_tools_export_jsonl.py:32 (RIGHT): test_export_jsonl_success only asserts the five hardcoded fields; it never verifies that additional article fields survive the export. This test gap allowed the data-loss bug to pass.
Adversarial validation
tools/export_jsonl.py:33 (RIGHT)confirmed: Article fields beyond the five hardcoded keys are preserved in the JSONL output. — Source-traced: the record dict is constructed with only document_id, page_number, article_id, headline, and body_blocks; any other keys in the article dict are discarded. The open review thread at line 39 confirms this behavior.tests/test_tools_export_jsonl.py:10 (RIGHT)confirmed: The success test verifies full article field preservation using the real ParseResponse schema. — The fixture contains string page/article entries and only the five fields; the test asserts only those five fields. No assertion covers images, captions, footnotes, or bounding boxes, so the test would pass even if those fields were dropped.- Residual risk: The tool will continue to silently drop article fields until record construction is changed to preserve all fields; the current test suite will not catch this because the fixture omits those fields.
Findings
- [high] tools/export_jsonl.py:33 (RIGHT): export_jsonl silently drops all article fields except document_id, page_number, article_id, headline, and body_blocks. This is a data-loss regression for the tool's stated purpose. Use **article (or explicitly include all known fields) to preserve images, captions, footnotes, bounding boxes, and any future fields.
- [medium] tests/test_tools_export_jsonl.py:10 (RIGHT): The test fixture does not reflect the real ParseResponse schema (it uses string page/article entries and only five fields) and does not assert preservation of images, captions, footnotes, or bounding boxes. Add a test with a realistic article containing these fields to prevent regressions.
- Result: REQUEST_CHANGES
- Head SHA:
4384d55e003ddcf7d7a39a09d7e580de54e517bf - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
1 participant
NewsDOM JSON 형식의 데이터를 기사 단위로 분리하여 한 줄씩 JSON 객체로 저장하는
tools/export_jsonl.py를 새롭게 개발하였습니다. 이를 통해 기사 단위의 대용량 데이터 분석 및 처리가 용이해집니다. 관련된 테스트 코드를 작성하였고 100% 커버리지를 확인했습니다.PR created automatically by Jules for task 14058315870946212790 started by @seonghobae