Potential risk: normalize_direction() silently remaps unknown aliases to 'forward' with no warning (commit f71bad5) #377
kapil971390
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Bug Report
Commit:
f71bad5— fix(skills/understand): persist canonical edge direction during mergeWhat was fixed
The PR correctly canonicalizes edge direction values (
both→bidirectional, case normalization, invalid →forward) before merging graphs, fixing duplicate edges leaking into the finalknowledge-graph.json.Remaining risk
normalize_direction()silently converts any unknown value toforwardwith no warning:If a batch graph ships an edge with a new valid alias (e.g.,
"reverse"or"both-ways") that is not yet in_DIRECTION_ALIASES, it silently becomes"forward"— changing the semantic meaning of the edge with no log entry.Impact scenario
A future LLM-generated batch may emit
"direction": "reverse"(a reasonable alias for"backward"). With the current code:"reverse"is not in_DIRECTION_ALIASES"reverse"is not in_VALID_DIRECTIONS"forward"— opposite directionSuggested fix
A warning here costs nothing and makes data quality issues visible during merges.
Beta Was this translation helpful? Give feedback.
All reactions