satore: fix contract-violation crash in unification-tree-ref - #739
Open
PratikDhanave wants to merge 1 commit into
Open
satore: fix contract-violation crash in unification-tree-ref#739PratikDhanave wants to merge 1 commit into
PratikDhanave wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Dooray! 메일 발송 실패 안내
메일 발송
실패 안내
***@***.***)
님께
보낸
메일이
전송되지
못하였습니다.
실패 사유를 확인해보세요.
* 받는 사람 :
***@***.***)
* 발송 시간 :
2026-07-25T13:31:47
* 메일 제목 :
[google-deepmind/deepmind-research] satore: fix contract-violation crash in unification-tree-ref (PR #739)
* 실패 사유 :
받는 사람이 현재 메일 수신을 제한하고 있습니다.
이 설정은 개인 또는 조직의 메일 수신 정책에 의해 적용되었습니다.
이 메일은 발신전용으로 회신되지 않습니다.
더 궁금하신 사항은
***@***.***
으로 문의해 주시기 바랍니다.
© Dooray!.
|
|
Dooray! 메일 발송 실패 안내
메일 발송
실패 안내
***@***.***)
님께
보낸
메일이
전송되지
못하였습니다.
실패 사유를 확인해보세요.
* 받는 사람 :
***@***.***)
* 발송 시간 :
2026-07-25T13:32:02
* 메일 제목 :
Re: [google-deepmind/deepmind-research] satore: fix contract-violation crash in unification-tree-ref (PR #739)
* 실패 사유 :
받는 사람이 현재 메일 수신을 제한하고 있습니다.
이 설정은 개인 또는 조직의 메일 수신 정책에 의해 적용되었습니다.
이 메일은 발신전용으로 회신되지 않습니다.
더 궁금하신 사항은
***@***.***
으로 문의해 주시기 바랍니다.
© Dooray!.
|
`unification-tree-ref` mapped the `utree-leaf-Clause` accessor directly over
`(trie-ref utree lit)`, but `trie-ref` returns a list of node-values where each
node-value is itself a list of `utree-leaf`. Mapping the struct accessor over
those inner lists raises a contract violation on any non-empty match.
The inline comment ("hence the append*") and the sibling function
`unification-tree-Clauses` both show the intended order: flatten with `append*`
first, then map the accessor. Swap the two operations to match.
The bug was latent because the exported function currently has no in-repo
callers, so no test exercised it.
PratikDhanave
force-pushed
the
fix/satore-unification-tree-ref
branch
from
July 25, 2026 04:45
2080ecf to
04ef486
Compare
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
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
unification-tree-refinsatore/unification-tree.rktcrashes on any non-empty match due to a swappedmap/append*.trie-refreturns a list of node-values, and each node-value is itself a list ofutree-leaf(seeadd-Clause!, which consesutree-leafs onto the node value). The buggy code mapped theutree-leaf-Clausestruct accessor directly over that outer list:so the accessor is handed an inner list rather than a
utree-leaf, raisingutree-leaf-Clause: contract violation, expected: utree-leaf?, given: '(#<utree-leaf>).Both the function's own inline comment ("hence the
append*") and the siblingunification-tree-Clauses((map utree-leaf-Clause (append* ...))) show the intended order: flatten first, then map. Fix swaps the two:The bug was latent because this exported function has no in-repo callers, so no test exercised it.
Testing
Verified by static tracing of
trie-ref/make-proc-tree-refand theutree-leafnode-value shape. Reviewers can validate viaraco test satore/tests/.