Skip to content

feat: tombstones record excludedBy for provenance tracking#420

Merged
vic merged 1 commit intodenful:mainfrom
sini:feat/tombstone-excludedBy
Apr 9, 2026
Merged

feat: tombstones record excludedBy for provenance tracking#420
vic merged 1 commit intodenful:mainfrom
sini:feat/tombstone-excludedBy

Conversation

@sini
Copy link
Copy Markdown
Collaborator

@sini sini commented Apr 9, 2026

Tombstones now carry meta.excludedBy — the name of the aspect whose meta.adapter caused the exclusion. Combined with the existing meta.provider (structural origin from provides chain), tombstones clearly document both who defines an aspect and who excluded it.

Tombstone meta fields:

  • provider — who defines this aspect (provides chain)
  • excludedBy — who excluded it (adapter source)
  • originalName — display name before ~prefix
  • replacedBy — replacement name (substitutions only)

Tombstones now carry meta.excludedBy — the name of the aspect whose
meta.adapter caused the exclusion. Combined with the existing
meta.provider (structural origin from provides chain), tombstones
clearly document both who defines an aspect and who excluded it.

Tombstone meta fields:
- provider    — who defines this aspect (provides chain)
- excludedBy  — who excluded it (adapter source)
- originalName — display name before ~prefix
- replacedBy  — replacement name (substitutions only)
# name collisions with live aspects. Harmless to module, visible to trace.
# Consumers should check meta.excluded before accessing other aspect fields.
# Tombstone fields:
# meta.excluded — true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we need meta.excluded ? given it is not present when false, we could be testing for meta ? excludedBy

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh but it could be also replacedBy

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

so is excluded = true when either replaced or excluded ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It could also be used outside of our specific chain.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

While it could be derived by either field, it does simplify validation when graph generating.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

One is a type tag, the other id debug/trace detail.

excludedBy is probably the wrong name -- I'll update this to excludedFrom, since it's where the tombstone is included

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh -- you already merged. I'll sneak it into my demo trace -> mermaid graph demo template since it has no consumers, unless you want a micro PR. :)

@vic vic added allow-ci allow all CI integration tests merge-approved Approved, merge yourself when ready labels Apr 9, 2026
@vic vic merged commit 3828d77 into denful:main Apr 9, 2026
49 of 80 checks passed
@sini sini deleted the feat/tombstone-excludedBy branch April 14, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted allow-ci allow all CI integration tests merge-approved Approved, merge yourself when ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants