[PandaHUN777]

Add a sentinel_ancestors JSON field for pure sentinel chains while keeping the existing sentinels field unchanged. Closes #59. Validation: go test ./..., go test -race -covermode=atomic -coverprofile=/tmp/errx-59-coverage.out ./..., go build ./..., go vet ./..., golangci-lint run, govulncheck ./..., git diff --check.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: af979268-4d4c-408d-affe-920c938df4bf

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[denisvmedia]

Reviewed b7f68e4 against #59 and the related decoder proposal in #37.

The first option in #59 is a reasonable choice for v1: keep sentinels unchanged and add a flattened, deduplicated sentinel_ancestors field. This addresses the missing parent classifications in logs without changing the existing field's type or meaning. I would not require replacing it with structured sentinels in this PR. The public accessor also follows the approach proposed in the issue.

There is an important scope distinction: the new field preserves ancestor membership, not the parent-child graph. It is useful input for a future registry-backed decoder, but it does not provide lossless hierarchy reconstruction or restore local sentinel identity on its own.

Requesting changes for the repeated chain-wide displayability scans in ancestor collection. The additional regression-coverage and documentation comments are non-blocking.

Validation: source inspection. The Go Test and Go Lint workflows for this commit are green; I have not run the repository test suite locally.

[denisvmedia]

[P2] Avoid rewalking every parent's subtree during ancestor collection

isPureSentinel calls errx.IsDisplayable, which performs a chain-wide errors.As search. Calling it for every parent re-traverses each remaining subtree; the visited map only guards addSentinelParents, not those nested searches.

A concrete valid DAG is built by repeatedly assigning current = errx.NewSentinel(label, leaf, current), with a shared leaf sentinel as the first parent. A single purity check is linear on this shape, but checking every remaining ancestor's subtree makes the new collection pass quadratic. WithMaxDepth does not bound this work when the hierarchy is attached as a classification to a plain error.

Please avoid recursive displayability detection at every parent. Once the root has passed the chain-wide displayable check, propagate that result and use shallow attribute/trace-kind filtering during descent, or compute the relevant properties in one traversal. Add a scaling benchmark for this DAG shape to guard against repeated subtree scans.

[denisvmedia]

Non-blocking: extend coverage for the new ancestor contract

This diamond tests deduplication of a shared parent object, but not deduplication by text across distinct objects. Please add two different NewSentinel("same", ...) parents, one pointing to a and the other to b; the expected ancestors are [same, a, b]. Both subtrees must still be visited even though their parent texts coincide.

Please also cover Marshal(child) directly, which exercises the separate node path in sentinelAncestorsForLevel, and mixed sentinel/attribute/trace/displayable parents to lock down the new field's filtering behavior. Existing assertions about Sentinels do not verify SentinelAncestors. These are regression-coverage requests, not claims that the current implementation fails those cases.

[denisvmedia]

Non-blocking: document the remaining loss of hierarchy information

The additive format is appropriate for the v1 option in #59, but this limitations entry should explicitly say that it preserves a flattened set of ancestor texts, not parent-child relationships. With multiple direct sentinels, a consumer cannot determine which ancestor belongs to which direct sentinel; intermediate edges and distinct identities sharing the same text are also not represented.

Please retain that limitation here and distinguish it from the future work in #37: a registry can resolve known texts to local sentinel values for matching, but this payload alone cannot reconstruct the original graph. No schema redesign is needed for this PR.