ENB: what to do about p._parentVnode?

11 views
Skip to first unread message

Edward K. Ream

unread,
Jul 2, 2026, 5:58:09 AM (yesterday) Jul 2
to leo-editor
​This Engineering Notebook post discusses a recent change to one of the Position class's fundamental low-level methods: p._parentVnode.

This change is now part of PR #4767. I believe the change is necessary, but I want to discuss my thoughts publicly.

tl;dr: p._parentVnode must raise ValueError when p has no valid parent VNode. Doing so is pythonic and helps prevent data loss. It also simplifies the method's return annotation.

Background

mypy rightly drew my attention to p._parentVnode. It is annotated to return a VNode, yet the legacy version of the routine may also return None!

More than a dozen of Leo's core methods call p._parentVnode, including four low-level position methods. None of those low-level methods check the value returned by p._parentVnode!!

This creates a mystery: why hasn't Leo been crashing?

First thoughts

This mystery absolutely must not be ignored.

Papering over the mystery, say by fudging p._parentVnode's annotated return value, would be a dereliction of duty. The point of the strict_optional mypy setting isn't prissy cleanliness. It's a deeper understanding of Leo's code, especially the Position and VNode classes.

Traces show that p._parentVnode never returns None, which is a pragmatic explanation of why Leo doesn't routinely crash. But relying on that kind of explanation amounts to relying on hope. Something more rigorous is required.

My first thought was that I needed a proof that p._parentVnode never returns None. But where to start? There are too many callers within Leo itself.

The solution

After long thought, I think that  p._parentVnode must raise ValueError in the following unusual situations:

- p is empty (p.v is None).
- p.v.context is empty (p is unattached to any outline).
- p.v is already c.v.context.hiddenRootNode.

Raising ValueError is best for several reasons:

- ValueError is an accurate description of the erroneous situations listed above.
- Raising some kind of exception is the best way to prevent data loss.

Laboriously testing the results of p._parentVnode would be unpythonic. The crucial low-level callers would have to raise another exception anyway! They would have no other way to ensure that their callers would handle the error correctly!

Summary

It would be intolerable to leave p._parentVnode as it was.

Raising ValueError is the best way to handle unexpected (invalid) arguments to p._parentVnode.

Raising ValueError also informs mypy that p._parentVnode expects to return only (valid) VNodes.

This new version of p._parentVnode joins the short list of actual code changes in PR #4767.

All of your comments, questions, and suggestions are welcome.

Edward

Thomas Passin

unread,
Jul 2, 2026, 8:45:28 AM (yesterday) Jul 2
to leo-editor
Are tests like `if p:` still going to evaluate as before?

Edward K. Ream

unread,
Jul 2, 2026, 10:34:48 AM (23 hours ago) Jul 2
to leo-e...@googlegroups.com
On Thu, Jul 2, 2026 at 7:45 AM Thomas Passin <tbp1...@gmail.com> wrote:

Are tests like `if p:` still going to evaluate as before?

Yes, because of the p.__eq__ and p.__ne__ methods.

I'll say more about a related question in a new ENB, coming soon.

Having the PR deal only with leoNodes.py has been a great idea. Furthermore, I'll wait about a week after merging the PR before merging any other PR.

Edward
Reply all
Reply to author
Forward
0 new messages