Skip to content

Commit 547910d

Browse files
rdmmfclaude
andcommitted
fix(bin-diff): show side B filename instead of its md5
parseRestfulPath decoded `collection` but left `coll_b` percent-encoded, so a collection name containing a space produced a `{coll}:file:{md5}:meta` lookup miss for side B; the strip then fell back to rendering the raw md5. Decode coll_b in all three `vs` branches. While tracing it: get_bin_sim returned the stored pair's own A/B ordering (canonical md5 sort, or pool build order) rather than the requested one, so opening a pair "backwards" swapped file metadata, unique_to_a/b and func_a/b. Re-orient the doc to the caller's order after loading it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 45ed375 commit 547910d

3 files changed

Lines changed: 77 additions & 9 deletions

File tree

bsimvis/app/routes/bin_sim.py

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,29 @@ def rebuild_bin_sim():
198198
}
199199

200200

201+
def _swap_side_keys(d):
202+
"""Swap every `<x>_a`/`<x>_b` (and `_1`/`_2`) pair in a dict, in place."""
203+
for key in list(d):
204+
if key.endswith("_a") and key[:-1] + "b" in d:
205+
twin = key[:-1] + "b"
206+
elif key.endswith("_1") and key[:-1] + "2" in d:
207+
twin = key[:-1] + "2"
208+
else:
209+
continue
210+
d[key], d[twin] = d[twin], d[key]
211+
212+
213+
def _flip_diff_sides(diff_data):
214+
"""Mirror a stored bin_sim doc so side A becomes side B and vice versa."""
215+
_swap_side_keys(diff_data)
216+
diff = diff_data.get("diff")
217+
if not isinstance(diff, dict):
218+
return
219+
_swap_side_keys(diff)
220+
for row in diff.get("matched", []):
221+
_swap_side_keys(row)
222+
223+
201224
def get_bin_sim(collection=None, md5_a=None, md5_b=None, coll_b=None, pool_id=None):
202225
"""Retrieve binary similarity diff for a pair."""
203226
if collection is None:
@@ -217,6 +240,9 @@ def get_bin_sim(collection=None, md5_a=None, md5_b=None, coll_b=None, pool_id=No
217240

218241
r = get_redis()
219242
coll_a = collection
243+
# What the caller asked for. Lookup below may reorder these to reach the
244+
# stored doc; the response must still come back in the caller's order.
245+
req_coll_a, req_md5_a, req_coll_b, req_md5_b = coll_a, md5_a, coll_b, md5_b
220246

221247
if pool_id:
222248
# For pool pairs, look up the SID via the 'involves' index to avoid
@@ -256,12 +282,14 @@ def get_bin_sim(collection=None, md5_a=None, md5_b=None, coll_b=None, pool_id=No
256282

257283
diff_data = json.loads(data_raw) if not isinstance(data_raw, dict) else data_raw
258284

259-
# Resolve actual coll_a/coll_b from the stored doc for metadata lookups
260-
if pool_id:
261-
coll_a = diff_data.get("coll_1") or coll_a
262-
coll_b = diff_data.get("coll_2") or coll_b
263-
md5_a = diff_data.get("md5_1") or md5_a
264-
md5_b = diff_data.get("md5_2") or md5_b
285+
# A pair is stored once, in whatever order it was built (canonical md5 sort for
286+
# collection pairs, build order for pools). Re-orient the doc to the requested
287+
# order so every "_a"/"_b" — file metadata, unique_to_b, func_b — describes the
288+
# binary the caller called A/B.
289+
stored_a = diff_data.get("md5_1") or diff_data.get("md5_a")
290+
if stored_a and stored_a != req_md5_a:
291+
_flip_diff_sides(diff_data)
292+
coll_a, md5_a, coll_b, md5_b = req_coll_a, req_md5_a, req_coll_b, req_md5_b
265293

266294
# Extract all unique function IDs
267295
fids = set()

bsimvis/app/static/js/utils.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ function parseRestfulPath() {
187187
params.view = 'file';
188188
} else if (parts[pIdx] === 'vs') {
189189
params.view = 'bin_sim';
190-
params.coll_b = parts[pIdx + 1];
190+
params.coll_b = decodeURIComponent(parts[pIdx + 1] || '');
191191
params.md5_b = parts[pIdx + 2];
192192
} else if (parts[pIdx] === 'functions' || parts[pIdx] === 'function') {
193193
pIdx++;
@@ -202,7 +202,7 @@ function parseRestfulPath() {
202202
params.view = 'function_features';
203203
} else if (parts[pIdx] === 'vs') {
204204
params.view = 'diff';
205-
params.coll_b = parts[pIdx + 1];
205+
params.coll_b = decodeURIComponent(parts[pIdx + 1] || '');
206206
params.md5_b = parts[pIdx + 2];
207207
params.addr_b = parts[pIdx + 3];
208208
params.id1 = `${stripPoolPrefix(params.collection || '')}:func:${params.md5}:${params.address}`;
@@ -234,7 +234,7 @@ function parseRestfulPath() {
234234
} else if (parts[pIdx] === 'vs') {
235235
params.view = 'diff';
236236
params.id1 = `${stripPoolPrefix(params.collection || '')}:func:${params.md5}:${params.address}`;
237-
params.coll_b = stripPoolPrefix(parts[pIdx + 1]) || '';
237+
params.coll_b = stripPoolPrefix(decodeURIComponent(parts[pIdx + 1] || '')) || '';
238238
params.md5_b = parts[pIdx + 2];
239239
params.addr_b = parts[pIdx + 3];
240240
params.id2 = `${stripPoolPrefix(params.coll_b || '')}:func:${params.md5_b}:${params.addr_b}`;

test_bin_sim_orientation.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
"""A bin_sim pair is stored once, in one order; the API must serve it in the
2+
order the caller asked for. Guards the side-flip used by get_bin_sim()."""
3+
4+
from bsimvis.app.routes.bin_sim import _flip_diff_sides
5+
6+
7+
def test_flip_diff_sides():
8+
doc = {
9+
"md5_a": "x",
10+
"md5_b": "y",
11+
"coverage_a": 1,
12+
"coverage_b": 2,
13+
"score": 9,
14+
"diff": {
15+
"unique_to_a": ["ua"],
16+
"unique_to_b": ["ub1", "ub2"],
17+
"matched": [{"func_a": "fa", "func_b": "fb", "similarity": 0.5}],
18+
},
19+
}
20+
21+
_flip_diff_sides(doc)
22+
assert (doc["md5_a"], doc["md5_b"]) == ("y", "x")
23+
assert (doc["coverage_a"], doc["coverage_b"]) == (2, 1)
24+
assert doc["score"] == 9
25+
assert doc["diff"]["unique_to_a"] == ["ub1", "ub2"]
26+
assert doc["diff"]["unique_to_b"] == ["ua"]
27+
assert doc["diff"]["matched"][0] == {
28+
"func_a": "fb",
29+
"func_b": "fa",
30+
"similarity": 0.5,
31+
}
32+
33+
_flip_diff_sides(doc)
34+
assert doc["md5_a"] == "x"
35+
assert doc["diff"]["unique_to_b"] == ["ub1", "ub2"]
36+
37+
38+
if __name__ == "__main__":
39+
test_flip_diff_sides()
40+
print("ok")

0 commit comments

Comments
 (0)