Skip to content

Accept dangling pointers and cut parse memory - #16

Merged
DavidMStraub merged 2 commits into
mainfrom
lenient-pointers-and-memory
Sep 30, 2026
Merged

DavidMStraub merged 2 commits into
mainfrom
lenient-pointers-and-memory

Conversation

@DavidMStraub

Copy link
Copy Markdown
Owner

Dangling pointers

loads()/load() no longer raise GedcomParseError for a pointer that matches no record. The pointer is kept as written, and validate() reports each one as a dangling-pointer error with its path (e.g. @F1@ FAM > WIFE).

Until now, one broken pointer stopped the whole parse, so a consumer such as gramps-gedcom7 could neither list all broken pointers nor import the rest of the file. Duplicate cross-reference identifiers still raise, because with duplicates the parser cannot tell which record a pointer means.

Behaviour change: code that relied on loads() to reject such files must now call validate().

Memory

  • GedcomStructure is a slots=True dataclass.
  • Each tag is interned, so all lines with the same tag share one string.
  • Lines are read one at a time instead of building a list of every line first.

Measured on a synthetic 35 MB file (100,000 individuals, 1.8M lines):

Before After
Peak during parse 627 MB 332 MB
Parsed tree left afterwards 472 MB 328 MB
Time 3.85 s 3.70 s

Behaviour change: because of __slots__, GedcomStructure instances no longer accept attributes beyond their fields, have no __dict__, and cannot be weakly referenced.

Tests

  • The test that expected the raise now checks that a dataset with three dangling pointers and one @VOID@ parses, and that validate() reports exactly the three.
  • The new line splitter is checked against the old re.split behaviour on every string over a, \r, \n up to length 7.

🤖 Generated with Claude Code

loads() no longer raises for a pointer that matches no record; validate()
reports each one as a dangling-pointer error. GedcomStructure uses slots,
tags are interned, and lines are read one at a time, halving peak memory.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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.

Copilot review overview

🟡 Changes recommended

Validation currently misses dangling pointers on extension and non-pointer-typed structures.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Updates parsing to preserve dangling pointers while reducing memory usage.

Changes:

  • Defers dangling-pointer detection to validation.
  • Streams line splitting, interns tags, and slots structures.
  • Adds documentation and regression coverage.
File Description
gedcom7/​parser.py Reduces parsing allocations and preserves unresolved pointers.
gedcom7/​types.py Enables dataclass slots.
test/​test_parser.py Tests line-splitting equivalence.
test/​test_conformance.py Tests dangling-pointer behavior.
README.md Documents the behavior change.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread gedcom7/parser.py
validate() checked a pointer only where the specification gives the
structure a pointer payload. Now that loads() keeps pointers to missing
records, a pointer on an extension, on a value or no-payload structure,
or on a record must be reported too. The target's record type is still
checked only where the payload names one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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.

Copilot review overview

🟢 Approval recommended

The implementation consistently matches the documented behavior and includes focused regression coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@DavidMStraub
DavidMStraub merged commit e2d7d2a into main Sep 30, 2026
7 checks passed
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.

2 participants