Skip to content

merge/irr: harden parsing and deterministic output#137

Open
SuvidhJ wants to merge 1 commit into
asmap:masterfrom
SuvidhJ:suvidhj/followup-irr-merge-hardening
Open

merge/irr: harden parsing and deterministic output#137
SuvidhJ wants to merge 1 commit into
asmap:masterfrom
SuvidhJ:suvidhj/followup-irr-merge-hardening

Conversation

@SuvidhJ

@SuvidhJ SuvidhJ commented Apr 18, 2026

Copy link
Copy Markdown

Small hardening follow-up from regression checks.

Main changes:

  1. fix IRR entry splitting for blank-line handling and final-entry flush
  2. make merge output writing deterministic and avoid blank-line artifacts
  3. cap merge worker count for safer behavior on constrained systems
  4. add deterministic merge regression coverage

Validation:
-> pytest tests/test_merge.py tests/test_irr_parse.py -q
-> pytest -q

Issue linkage:

Copilot AI review requested due to automatic review settings April 18, 2026 17:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens IRR parsing and merge behavior to better handle edge-case inputs and produce stable merge outputs under constrained resources (addresses #134).

Changes:

  • Fix IRR RPSL object splitting to handle blank lines robustly and flush the final object without requiring a trailing blank line.
  • Make merge output writing more deterministic and avoid blank-line artifacts; cap merge worker concurrency.
  • Add a regression test asserting deterministic merge output.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
tests/test_merge.py Adds a regression test to ensure general_merge produces identical output on repeated runs.
kartograf/merge.py Caps ProcessPoolExecutor workers, adjusts chunk sizing accordingly, and rewrites output emission to avoid blank-line artifacts and improve determinism.
kartograf/irr/parse.py Improves IRR entry splitting logic for blank-line handling and ensures the final entry is flushed even without a trailing blank line.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread kartograf/irr/parse.py
Comment thread kartograf/merge.py Outdated
Comment thread kartograf/merge.py Outdated
Comment thread tests/test_merge.py
@SuvidhJ SuvidhJ force-pushed the suvidhj/followup-irr-merge-hardening branch from d6e92b3 to 735af53 Compare April 18, 2026 18:40
@fjahr

fjahr commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

Please try to explain what the problem(s) are that you are solving in the PR rather than describing only the change. You should also make a separate commit for each fix, ideally with a test that did not pass before the fix but passes afterwards.

Validation:

Please also test consistency with a full reproduction run from asmap-data, thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants