Skip to content

lore-proto, lore-revision, lore-server: opt-in last-commit attribution on RevisionTree - #179

Open
lorelab-graham wants to merge 1 commit into
EpicGames:mainfrom
lorelab-graham:tree_attribution
Open

lore-proto, lore-revision, lore-server: opt-in last-commit attribution on RevisionTree#179
lorelab-graham wants to merge 1 commit into
EpicGames:mainfrom
lorelab-graham:tree_attribution

Conversation

@lorelab-graham

Copy link
Copy Markdown

Summary

Adds TreeNode.last_commit to ThinClientService.RevisionTree, so a client rendering a directory listing can show per-entry last-commit message, timestamp, and revision identifier without walking history client-side. Off by default; existing callers pay nothing.

Without this, a client needs RevisionDiff per revision back from the tip plus RevisionInfo per touching revision to produce the same rendering.

Attribution rides on the per-entry back-pointer that lore-revision already maintains on NodeFileMetadata for file::history. One delta-block read per state, one metadata-block read per entry.

  • New request field RevisionTreeRequest.include_last_commit gates the attribution work
  • TreePath gains last_revision and last_revision_repository, populated inside enumerate_children from TreeAttribution built against that call's walk_state / walk_repository
  • Attribution follows the walker across link boundaries. Each linked-subtree recursion builds its own TreeAttribution from the linked repository's state, so entries inside a link attribute against the linked repository's revisions rather than the parent's. NodeIDs are u32 indices - mixing states would silently return plausible garbage
  • Directories inherit their descendant's revision through the existing metadata propagation, so folder rows attribute directly without a max-over-descendants pass
  • Server handler deduplicates by (RepositoryId, Hash) and resolves each unique revision once via load_tree_commit, reusing the walked repository's context when possible and building a linked context only when the tree spans a link
  • Legacy v0 handler passes include_last_commit: false - the v0 message has no field to carry attribution

Test Plan

  • lore-revision unit tests: raw metadata invariant (per-entry back-pointer vs parent stamp), tree() end-to-end with attribution on and off, and cross-link attribution asserting the linked-subtree carries the linked repository's revision and id
  • lore-server unit tests: load_tree_commit signature-and-identifier round-trip; include_last_commit gates the field without changing which nodes are emitted; per-entry attribution does not borrow a neighbour's commit and does not collide across repositories
  • lore-proto shape test extended to destructure TreeCommit and the new fields on TreeNode / RevisionTreeRequest
  • New end-to-end tests in scripts/test/test_revision_tree_last_commit.py cover the flag gate, per-entry attribution across two revisions, cross-link attribution, and directory propagation on the wire
  • cargo test --workspace passes (no regressions)

…n on RevisionTree

## Summary

Adds TreeNode.last_commit to ThinClientService.RevisionTree, so a client rendering a directory listing can show per-entry last-commit message, timestamp, and revision identifier without walking history client-side. Off by default; existing callers pay nothing.

Without this, a client needs RevisionDiff per revision back from the tip plus RevisionInfo per touching revision to produce the same rendering.

Attribution rides on the per-entry back-pointer that lore-revision already maintains on NodeFileMetadata for file::history. One delta-block read per state, one metadata-block read per entry.

- New request field RevisionTreeRequest.include_last_commit gates the attribution work
- TreePath gains last_revision and last_revision_repository, populated inside enumerate_children from TreeAttribution built against that call's walk_state / walk_repository
- Attribution follows the walker across link boundaries. Each linked-subtree recursion builds its own TreeAttribution from the linked repository's state, so entries inside a link attribute against the linked repository's revisions rather than the parent's. NodeIDs are u32 indices - mixing states would silently return plausible garbage
- Directories inherit their descendant's revision through the existing metadata propagation, so folder rows attribute directly without a max-over-descendants pass
- Server handler deduplicates by (RepositoryId, Hash) and resolves each unique revision once via load_tree_commit, reusing the walked repository's context when possible and building a linked context only when the tree spans a link
- Legacy v0 handler passes include_last_commit: false - the v0 message has no field to carry attribution

Test Plan

- lore-revision unit tests: raw metadata invariant (per-entry back-pointer vs parent stamp), tree() end-to-end with attribution on and off, and cross-link attribution asserting the linked-subtree carries the linked repository's revision and id
- lore-server unit tests: load_tree_commit signature-and-identifier round-trip; include_last_commit gates the field without changing which nodes are emitted; per-entry attribution does not borrow a neighbour's commit and does not collide across repositories
- lore-proto shape test extended to destructure TreeCommit and the new fields on TreeNode / RevisionTreeRequest
- New end-to-end tests in scripts/test/test_revision_tree_last_commit.py cover the flag gate, per-entry attribution across two revisions, cross-link attribution, and directory propagation on the wire
- cargo test --workspace passes (no regressions)

Signed-off-by: Graham Plumb <graham@lorelab.io>
@github-actions github-actions Bot added area:server Server, provider integrations, telemetry area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals labels Aug 27, 2026
// Revision that last modified this entry, set only when the request asks
// for it. Absent wherever the server cannot attribute an entry - most
// commonly the repository root.
optional TreeCommit last_commit = 7;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since this field will be the same for potentially a large number of files I would rather have it in a separate list of unique revisions, and the tree node refer to the revision by index into the list.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since the revision list could be arbitrarily long, it should go as a separate Revision type in the RevisionTreeResponse so it can be streamed. And requirement that any Revision indexed by a TreeNode must have appeared before the tree node in the stream.


// The revision that last modified a tree entry. Directories report the
// revision of the most recent change anywhere beneath them.
message TreeCommit {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of introducing a new type I would suggest using the Revision type in the thin client proto model

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

Labels

area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals area:server Server, provider integrations, telemetry

Development

Successfully merging this pull request may close these issues.

2 participants