Skip to content

fix: isolate legacy Tinker branch contexts - #1114

Merged
bradhilton merged 1 commit into
mainfrom
tarkin/tinker-service-branch-masks
Oct 10, 2026
Merged

bradhilton merged 1 commit into
mainfrom
tarkin/tinker-service-branch-masks

Conversation

@bradhilton

@bradhilton bradhilton commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Agent: Tarkin

Part of #1111 (item 9).

Problem: The legacy Tinker service splits each packed row into one datum per branch. It built each branch's mask as (group == g) | (parent == parent(g)). Prefix-tree roots have parent_id == group_id, so that mask pulled sibling branches into every datum's context, and the root's datum covered the whole row. Targets were also shifted across the whole packed row before masking, so a branch's first target could be a sibling's token.

Behavior after this change: Each datum now contains exactly its branch and that branch's ancestors, read from the packer's own prefix-tree segments (parse_prefix_tree_row). Targets are shifted within each datum. Optimizer stepping is unchanged: ART's gradient accumulation is fixed at one packed sequence since #808, so there is still one optimizer step per packed row.

Validation:

  • New tests: tests/unit/test_tinker_service_training.py runs the real packer and loss callback against a fake Tinker client. It covers two siblings, a deeper tree, separate roots and padding, checking each datum's inputs, targets and weights, plus the per-row optimizer count. 3 of its 4 tests fail on main.
  • Existing tests: the Tinker service test suites pass (58 tests).

Limitations: Tested against a fake client only, with no live Tinker run. Prompt-only root datums are still sent, as before.

@bradhilton
bradhilton force-pushed the tarkin/tinker-service-branch-masks branch 2 times, most recently from bbfe7c6 to b55b15d Compare October 10, 2026 17:35
@bradhilton
bradhilton force-pushed the tarkin/tinker-service-branch-masks branch from b55b15d to c923cb4 Compare October 10, 2026 18:58
@bradhilton
bradhilton marked this pull request as ready for review October 10, 2026 19:26
@bradhilton
bradhilton merged commit 99f078b into main Oct 10, 2026
12 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.

1 participant