Skip to content

refactor(dom): iterate sorted page mappings through items - #818

Draft
seonghobae wants to merge 5 commits into
developfrom
bolt-optimize-dict-iteration-8530617915992516171
Draft

refactor(dom): iterate sorted page mappings through items#818
seonghobae wants to merge 5 commits into
developfrom
bolt-optimize-dict-iteration-8530617915992516171

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

변경 범위

_build_pages_without_page_idx_build_pages_with_page_idx에서 정렬된 dict key를 얻은 뒤 같은 mapping을 다시 조회하던 경로를 sorted(mapping.items()) 순회로 바꿉니다. 현재 authority는 behavior-preserving refactor candidate이며 buyer-visible 성능 개선이 아닙니다.

  • protected base: develop@e06b1f3fb10903569124af011da213951e6e2473
  • current exact head: 95c0d93e16f28af24eef5fc404ae073c200861ca
  • effective changed files: src/newsdom_api/dom_builder.py, tests/test_dom_builder.py
  • lifecycle: Draft / mergeable

branch-local performance doctrine과 자명한 Bolt 주석은 effective diff에 없습니다.

Correctness acceptance

Current head의 실제 build_dom() regressions가 다음을 고정합니다.

  • no-page-idx + multi-page metadata
  • sparse page_idx
  • mixed missing page_idx
  • metadata fallback
  • deterministic page ordering

Source-string assertion이 아니라 ParseResponse 결과를 검증합니다. 이 PR의 product acceptance는 protected implementation과의 동작 보존입니다.

Current-head evidence

Terminal GREEN on exact 95c0d93e16f28af24eef5fc404ae073c200861ca:

  • tests 34074105241
  • container-image 34074105306
  • repository codeql 34074105275
  • ClusterFuzzLite 34074105247
  • Scorecards 34074105240
  • SAST Semgrep 34074105273

Two independent blockers remain and neither should be worked around in this leaf PR.

1. Protected-base pypdf security prerequisite

Security Scan 34074105227, exact-head Trivy job 101610275500, checked out this exact SHA and reported three MEDIUM findings from the inherited uv.lock pypdf 6.15.0:

Canonical repair is newsdom-api#794@96a6a50017b8c2525a139492f856cc25541d241f, which raises the declared floor to >=6.16.1,<7.0, locks 6.16.2, and is terminal GREEN in its exact-head Security Scan. This PR must not copy or depend on that mutable sibling head. Merge #794 normally into protected develop, then adopt the protected descendant here by non-force restack and obtain fresh exact-head evidence.

2. Organization CodeQL terminal-publication prerequisite

CodeQL PR 34074105324 detects languages successfully. Both compatibility jobs dispatch successfully and then fail because no authenticated terminal verdict is available:

  • actions 101610776743
  • python 101610776787

The exact canary is already tracked by canonical owner ContextualWisdomLab/.github#1929. Native repository CodeQL success is not a substitute.

Performance claim boundary

The original benchmark claim remains withdrawn. A performance claim requires the actual DOM build path, representative/right-cleared parser payload, identical Python/runtime/CPU, protected-vs-candidate comparison, warm-up/cardinality, median/p95 and allocation/CPU profile. Removing a dict re-lookup alone is not buyer-visible evidence.

Keep Draft until the pypdf repair is part of protected develop, this branch is normally restacked on that immutable base, all exact-head security/CodeQL gates are terminal, and qualifying current-head review is clean. No predecessor result transfer, mutable sibling dependency, dummy commit, self-approval, force push, scanner suppression, or gate weakening.

@google-labs-jules

Copy link
Copy Markdown

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: d03fab90-97dd-4c06-9d5f-4d891fc06118

📥 Commits

Reviewing files that changed from the base of the PR and between e06b1f3 and 9e812b1.

📒 Files selected for processing (1)
  • src/newsdom_api/dom_builder.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

두 페이지 빌더가 정렬된 딕셔너리 항목을 직접 순회하도록 변경되었습니다. 별도의 키 조회를 제거했으며, 동작은 변경되지 않았습니다.

Changes

페이지 빌더 반복문 정리

Layer / File(s) Summary
정렬된 항목 직접 순회
src/newsdom_api/dom_builder.py
_build_pages_without_page_idx_build_pages_with_page_idx가 정렬된 items() 결과에서 키와 값을 직접 가져옵니다. 기존의 반복별 딕셔너리 조회를 제거했습니다.

Estimated code review effort: 1 (매우 낮음) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 9e812

Page construction now iterates sorted dictionary entries directly, preserving page ordering and content selection while avoiding unnecessary lookups. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed 설명은 변경 범위, 정확성 기준, 검증 결과, 보안 및 CodeQL 차단 요인, 후속 조치를 충분히 설명합니다. 템플릿의 명시적 Git Flow target 항목과 검증 체크박스 형식은 없지만, 관련 정보는 대부분 제공되어 있습니다.
Title check ✅ Passed 제목은 dom_builder.py의 정렬된 페이지 매핑을 .items()로 순회하도록 변경한 핵심 리팩터링을 정확하고 간결하게 설명합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bolt-optimize-dict-iteration-8530617915992516171

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 6, 2026
@seonghobae seonghobae added priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: maintenance Maintenance, build, dependency, or operational upkeep labels Sep 6, 2026 — with ChatGPT Codex Connector
@seonghobae
seonghobae marked this pull request as draft September 7, 2026 00:14
@seonghobae seonghobae changed the title ⚡ Bolt: 딕셔너리 순회 최적화를 통한 성능 개선 refactor(dom): iterate sorted page mappings through items Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

@jules Please repair this Draft on ordinary descendants of exact head 9e812b150f29de71dce0eafe4dddfcd8fff867e5: remove the two # ⚡ Bolt: Iterate over items() comments because they only restate the code, then add/confirm behavioral regression coverage that proves page construction is unchanged for no-page-idx with multi-page metadata, sparse page_idx, mixed missing page_idx, metadata fallback, and deterministic ordering. The RED/GREEN oracle must inspect the resulting ParseResponse, not source text. Do not reintroduce .jules/bolt.md. Do not claim buyer-visible performance without an actual protected-vs-candidate DOM-build measurement on representative/right-cleared parser payloads with identical runtime/CPU plus warm-up, cardinality, median/p95 and allocation/CPU profile. Preserve history with non-force descendants and regenerate exact-head checks.

@seonghobae seonghobae removed the status: needs-review Open pull request requiring current-head review or checks label Sep 7, 2026
@seonghobae seonghobae added the status: draft Draft pull request label Sep 7, 2026 — with ChatGPT Codex Connector
@google-labs-jules

Copy link
Copy Markdown

@jules Please repair this Draft on ordinary descendants of exact head 9e812b150f29de71dce0eafe4dddfcd8fff867e5: remove the two # ⚡ Bolt: Iterate over items() comments because they only restate the code, then add/confirm behavioral regression coverage that proves page construction is unchanged for no-page-idx with multi-page metadata, sparse page_idx, mixed missing page_idx, metadata fallback, and deterministic ordering. The RED/GREEN oracle must inspect the resulting ParseResponse, not source text. Do not reintroduce .jules/bolt.md. Do not claim buyer-visible performance without an actual protected-vs-candidate DOM-build measurement on representative/right-cleared parser payloads with identical runtime/CPU plus warm-up, cardinality, median/p95 and allocation/CPU profile. Preserve history with non-force descendants and regenerate exact-head checks.

요청하신 대로 # ⚡ Bolt: Iterate over items() 주석을 제거하고 .jules/bolt.md 변경 사항을 롤백했습니다. 또한 DOM 생성 시 ParseResponse를 직접 검사하는 회귀 테스트(behavior regression coverage)를 tests/test_dom_builder.py에 추가하여, multi-page metadata 시의 no-page-idx 처리, sparse page_idx, mixed missing page_idx, fallback 동작 및 deterministic ordering에 대해 100% 커버리지로 안전성을 증명하도록 조치했습니다. 변경 사항은 모두 이전 commit 위에서 반영했습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance priority: medium Normal-priority or P2 work status: draft Draft pull request type: maintenance Maintenance, build, dependency, or operational upkeep

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant