feat(tree-edit): add pack-owned SET_PROPERTY action - #1067
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds ChangesSet-property action
Sequence Diagram(s)sequenceDiagram
participant TreeEditSpace
participant TreeEditPolicy
participant TreeEditDiffusionModel
participant ReachabilityHarness
TreeEditSpace->>TreeEditDiffusionModel: expose property domains
TreeEditDiffusionModel->>TreeEditPolicy: provide property-value head
TreeEditPolicy->>TreeEditDiffusionModel: return property-value logits
TreeEditDiffusionModel->>TreeEditSpace: apply valid set-property edit
ReachabilityHarness->>TreeEditSpace: enumerate property transitions
TreeEditSpace->>ReachabilityHarness: return successor states and paths
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/design/var1-02-set-property-action-20260726.md (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHeading names SLM-299/LAR1-03 while the document is the VAR1-02 / SLM-425 evidence.
Filename and content are the set-property delta; the inherited renderer title makes it easy to mistake this for the earlier SLM-299 audit artifact.
♻️ Suggested title
-# SLM-299 (LAR1-03): X22 edit-space reachability audit +# VAR1-02 (SLM-425): set-property action — X22 edit-space reachability audit🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/var1-02-set-property-action-20260726.md` at line 1, Update the document’s top-level heading to identify it as the VAR1-02 / SLM-425 set-property evidence, replacing the inherited SLM-299/LAR1-03 X22 edit-space reachability audit title. Keep the heading aligned with the filename and document content.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/var1-02-set-property-action-20260726.json`:
- Around line 488-496: Regenerate the evidence artifact from a clean working
tree so the version_stamp metadata records code_dirty as false while preserving
the appropriate clean code_commit, component versions, and generation timestamp.
In `@src/slm_training/dsl/variants.py`:
- Line 250: Update the tree-edit variant’s kernel_ops declaration to use a
dedicated rest-mutation operation identity instead of openui.set_property. Add
or register the narrower rest-edition operator and its contract, then reference
that new symbol in the relevant variant while preserving ACTION_SET_PROPERTY for
tree_edit_diffusion.
In `@tests/test_models/test_tree_edit_diffusion.py`:
- Around line 293-312: Add a focused test in
tests/test_models/test_tree_edit_diffusion.py for ACTION_SET_PROPERTY with
exactly one legal property value, asserting the selected value is returned
through the singleton bypass and the scorer’s forwards_count remains 0. Reuse
the existing model/edit fixtures and _enumerate_edits() or relevant decode
helper symbols, and keep the assertion deterministic without changing migration
coverage.
---
Nitpick comments:
In `@docs/design/var1-02-set-property-action-20260726.md`:
- Line 1: Update the document’s top-level heading to identify it as the VAR1-02
/ SLM-425 set-property evidence, replacing the inherited SLM-299/LAR1-03 X22
edit-space reachability audit title. Keep the heading aligned with the filename
and document content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e349f57-2206-4fb0-9174-cfc4e2e62a03
📒 Files selected for processing (10)
docs/design/var1-02-set-property-action-20260726.jsondocs/design/var1-02-set-property-action-20260726.mdsrc/slm_training/dsl/variants.pysrc/slm_training/harnesses/experiments/slm299_edit_reachability.pysrc/slm_training/models/checkpoint_migrate.pysrc/slm_training/models/tree_edit_diffusion.pysrc/slm_training/resources/variant_registry.jsonsrc/slm_training/resources/versions.jsontests/test_harnesses/experiments/test_slm299_edit_reachability.pytests/test_models/test_tree_edit_diffusion.py
Comment on lines
+488
to
+496
| "version_stamp": { | ||
| "stamp_schema": "version_stamp/v1", | ||
| "code_commit": "74e6cca3f84cae21bf2a4b11055f1c9495f144c9", | ||
| "code_dirty": true, | ||
| "components": { | ||
| "harness.experiments.slm299_edit_reachability": "v6" | ||
| }, | ||
| "stamped_at": "2026-07-26T19:34:30.301938+00:00" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Regenerate this evidence from a clean tree (code_dirty: true).
Committed evidence in this repository is normally published from a clean revision — several components' history notes explicitly record "publish the clean reproducible report … code_dirty=false" and one harness even gates evidence writes behind --allow-dirty. A dirty stamp makes the pinned code_commit non-reproducible, so this artifact cannot be replayed byte-for-byte.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/var1-02-set-property-action-20260726.json` around lines 488 -
496, Regenerate the evidence artifact from a clean working tree so the
version_stamp metadata records code_dirty as false while preserving the
appropriate clean code_commit, component versions, and generation timestamp.
| action_alphabet_id="tree_edit_diffusion.edit_actions", | ||
| action_alphabet_fingerprint=_alphabet_fingerprint(_tree_edit_action_names()), | ||
| kernel_ops=(), | ||
| kernel_ops=("openui.set_property",), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'set_property' --type=py -g 'src/slm_training/dsl/**'
rg -n 'openui.set_property' src/slm_training/resources/ops_vocab_registry.jsonRepository: Tyler-R-Kendrick/slm-training
Length of output: 248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect around the reported action identity plus nearby variant definitions.
wc -l src/slm_training/dsl/variants.py
sed -n '220,270p' src/slm_training/dsl/variants.py
# Search for the exact new action identity across tracked files relevant to DSL/resources.
rg -n "openui\.set_property|set_property|rest|finite pack|pack-owned|mutat" src/slm_training/dsl src/slm_training/resources --glob '*.py' --glob '*.json' | sed -n '1,200p'
# Locate OPS_VOCAB definition/load path.
rg -n "OPS_VOCAB|ops_vocab_registry|operator registry|set_property" src/slm_training -g '*.py' --glob '*.json' | sed -n '1,200p'Repository: Tyler-R-Kendrick/slm-training
Length of output: 32927
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how variants.py validates kernel_ops and how OPS_VOCAB is derived from the registry.
sed -n '70,135p' src/slm_training/dsl/variants.py
sed -n '35,105p' src/slm_training/dsl/ops_vocab.py
sed -n '35,70p' src/slm_training/resources/ops_vocab_registry.json
# Inspect the set_property local operator semantic description and mutation argument usage.
sed -n '1,90p' src/slm_training/dsl/operators/local.py
sed -n '380,605p' src/slm_training/dsl/operators/local.py
# Inspect tree-edit action source description for its realized operation and argument shape.
sed -n '1,140p' src/slm_training/dsl/tree_edit_diffusion.py
sed -n '140,260p' src/slm_training/dsl/tree_edit_diffusion.pyRepository: Tyler-R-Kendrick/slm-training
Length of output: 17305
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "tracked files matching tree_edit:"
git ls-files | rg 'tree_edit|edit_diffusion|edit' | sed -n '1,120p'
echo
echo "search for tree_edit_diffusion and action names"
rg -n "tree_edit_diffusion|_tree_edit_action_names|edit_actions|set_property|component_widen|rest" src/slm_training/dsl src/slm_training/harnesses/experiments src/slm_training/resources --glob '*.py' --glob '*.json' | sed -n '1,240p'Repository: Tyler-R-Kendrick/slm-training
Length of output: 37848
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "tree_edit_diffusion outlines/snippets"
for f in src/slm_training/models/tree_edit_diffusion.py src/slm_training/dsl/tree_edit_diffusion.py 2>/dev/null; do
if [ -f "$f" ]; then
wc -l "$f"
sed -n '1,180p' "$f"
printf '\n--- edit_actions/action id mentions in %s ---\n' "$f"
rg -n "ACTION_|ACTION_*|edit_actions|set_property|rest|component|payload|target" "$f" | sed -n '1,220p'
else
echo "missing: ${f}"
fi
done
echo
echo "tree edit action references in sources/tests"
rg -n '_tree_edit_action_names|tree_edit_diffusion\.edit_actions|edit_actions|ACTION_[A-Z]+|SET_PROPERTY|rest|container_rests|payload' src/slm_training src/tests tests tests/test_models --glob '*.py' | sed -n '1,260p'Repository: Tyler-R-Kendrick/slm-training
Length of output: 258
🏁 Script executed:
#!/bin/bash
set -u
echo "tree_edit_diffusion outlines/snippets"
for f in src/slm_training/models/tree_edit_diffusion.py src/slm_training/dsl/tree_edit_diffusion.py; do
if [ -f "$f" ]; then
wc -l "$f"
sed -n '1,180p' "$f"
printf '\n--- edit_actions/action id mentions in %s ---\n' "$f"
rg -n "ACTION_|ACTION_.*|edit_actions|set_property|rest|component|payload|target" "$f" | sed -n '1,220p'
else
echo "missing: ${f}"
fi
done
echo
echo "tree edit action references in sources/tests"
rg -n '_tree_edit_action_names|tree_edit_diffusion\.edit_actions|edit_actions|ACTION_[A-Z]+|SET_PROPERTY|rest|container_rests|payload' src/slm_training src/tests tests --glob '*.py' | sed -n '1,280p'Repository: Tyler-R-Kendrick/slm-training
Length of output: 50385
Use a rest-mutation op identity, not openui.set_property.
tree_edit_diffusion only declares ACTION_SET_PROPERTY for this action, while the registry/operator declaration for openui.set_property is a single node.role.value AST property edit. Claiming that as the tree-edit kernel op masks a broader pack-owned rest action; add/register a narrower rest-edition op name and contract instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slm_training/dsl/variants.py` at line 250, Update the tree-edit variant’s
kernel_ops declaration to use a dedicated rest-mutation operation identity
instead of openui.set_property. Add or register the narrower rest-edition
operator and its contract, then reference that new symbol in the relevant
variant while preserving ACTION_SET_PROPERTY for tree_edit_diffusion.
Comment on lines
+293
to
+312
| assert report["source_format_version"] == 2 | ||
| assert report["output_format_version"] == 3 | ||
| assert report["preserved_action_head_rows"] == 11 | ||
| assert (tmp_path / "ckpt_migrated.migrate.json").exists() | ||
| migrated = TreeEditDiffusionModel.from_checkpoint(out_path, device="cpu") | ||
| new_w = migrated.policy.action_head.weight | ||
| assert new_w.shape[0] == model.policy.action_head.weight.shape[0] | ||
| assert _torch.allclose(new_w[:4], old_w) | ||
| assert _torch.allclose(migrated.policy.action_head.bias[:4], old_b) | ||
| assert _torch.allclose(new_w[:11], old_w) | ||
| assert _torch.allclose(migrated.policy.action_head.bias[:11], old_b) | ||
| assert migrated.policy.property_value_head.out_features == 2 | ||
|
|
||
| # Existing action logits are bit-identical after the format migration. | ||
| state = model._state_batch([PROGRAM]) | ||
| ctx = _torch.zeros((1, 1, cfg.d_model)) | ||
| ctx_pad = _torch.zeros((1, 1), dtype=_torch.bool) | ||
| model.eval() | ||
| migrated.eval() | ||
| before = model.policy(state, model.tokenizer.pad_id, ctx, ctx_pad)["action"] | ||
| after = migrated.policy(state, migrated.tokenizer.pad_id, ctx, ctx_pad)["action"] | ||
| assert _torch.equal(before[:, :11], after[:, :11]) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for an existing singleton-bypass test covering the set-property decode path.
rg -nP -C4 'forwards_count' --type=py -g 'tests/**'
rg -nP -C4 'SET_PROPERTY|property_value_head' --type=py -g 'tests/**'Repository: Tyler-R-Kendrick/slm-training
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -u
echo "== find relevant files =="
git ls-files | rg '(^|/)test_tree_edit_diffusion\.py$|tree_edit|TreeEdit|property|diffusion|policy' | head -200
echo
echo "== all forwards_count occurrences =="
rg -n -C3 'forwards_count|singleton|bypass|I2|decode path' --type=py -g 'tests/**' -g '*.py' | head -250
echo
echo "== tree diff test relevant symbols =="
rg -n -C3 'SET_PROPERTY|property_value_head|action_logits|policy\(|eval\(|diffusion' tests tests -g '*.py' | head -300Repository: Tyler-R-Kendrick/slm-training
Length of output: 26369
🏁 Script executed:
#!/bin/bash
set -u
echo "== tracked python and test files containing tree edit symbols =="
git ls-files '*.py' | rg 'tree|edit|diffusion|model|policy|operator' | head -300
echo
echo "== direct occurrences across repo =="
rg -n -C3 'set_property|property_value|property_value_head|SET_PROPERTY|forwards_count|singleton|bypass|TreeEditDiffusionModel' --type=py --type=md --type=json | head -400Repository: Tyler-R-Kendrick/slm-training
Length of output: 14950
🏁 Script executed:
#!/bin/bash
set -u
echo "== locate diff tree model file =="
git ls-files | xargs -r grep -l 'TreeEditDiffusionModel\|property_value_head\|set_property\|SET_PROPERTY' 2>/dev/null | rg '\.py$|\.md$|\.json$' | head -200
echo
echo "== test file context =="
if [ -f tests/test_models/test_tree_edit_diffusion.py ]; then
wc -l tests/test_models/test_tree_edit_diffusion.py
sed -n '240,330p' tests/test_models/test_tree_edit_diffusion.py | cat -n
else
echo "tests/test_models/test_tree_edit_diffusion.py not found"
fi
echo
echo "== model file occurrences =="
for f in $(git ls-files '*tree*diffusion*.py' '*diffusion*tree*.py'); do
echo "--- $f ---"
rg -n -C5 'property_value|set_property|SET_PROPERTY|decode|forwards_count|singleton|bypass' "$f" || true
done
echo
echo "== all occurrences without type filters, limited =="
rg -n -C3 'property_value|set_property|SET_PROPERTY|forwards_count|singleton-bypass|bypass' --glob '*.py' --glob '*.md' --glob '*.json' --glob '*.js' --glob '*.ts' . | head -500Repository: Tyler-R-Kendrick/slm-training
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -u
echo "== decode path implementation =="
sed -n '1274,1430p' src/slm_training/models/tree_edit_diffusion.py | cat -n
echo
echo "== decode stats/forwards usage =="
rg -n -C3 'DecodeStats|forwards_count|policy_forward|eval_forward|_forward\(|action_value|property_value' src/slm_training/models/tree_edit_diffusion.py src/slm_training/models/decode_stats.py tests/test_models/test_tree_edit_diffusion.py | head -250
echo
echo "== test file tail =="
sed -n '315,365p' tests/test_models/test_tree_edit_diffusion.py | cat -nRepository: Tyler-R-Kendrick/slm-training
Length of output: 23898
Add a forwards_count == 0 singleton-bypass test for the set-property value channel.
_enumerate_edits() scores property_value candidates for ACTION_SET_PROPERTY, and this test file only covers migration/precondition semantics. I2 requires a deterministic singleton-bypass test for new decode paths, but there is no setter-value coverage that asserts zero forward scorer usage when only one legal property value is available.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_models/test_tree_edit_diffusion.py` around lines 293 - 312, Add a
focused test in tests/test_models/test_tree_edit_diffusion.py for
ACTION_SET_PROPERTY with exactly one legal property value, asserting the
selected value is returned through the singleton bypass and the scorer’s
forwards_count remains 0. Reuse the existing model/edit fixtures and
_enumerate_edits() or relevant decode helper symbols, and keep the assertion
deterministic without changing migration coverage.
Source: Coding guidelines
This was referenced Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ACTION_SET_PROPERTYaction with structured fragment validation, safe inverse supervision, decoder scoring, and format-2→3 migrationopenui.set_propertyin the variant contractVerification
pytest tests/test_models/test_tree_edit_diffusion.py tests/test_harnesses/experiments/test_slm299_edit_reachability.py tests/test_harnesses/experiments/test_var1_01_set_property_probe.py tests/test_dsl/test_variants.py -q(56 passed)python -m scripts.verify_decode_invariantspython -m scripts.verify_version_stamps --checkpython -m scripts.repo_policyruff checkandgit diff --checkCloses SLM-425.
Summary by CodeRabbit
New Features
restto be changed directly.Bug Fixes
Documentation