Simplify rewrite reporting to whole terms only
Change-Id: I412d6898efe31bb5d26435b3207133075f6fa3a5
diff --git a/cmd/koralmapper/main_test.go b/cmd/koralmapper/main_test.go
index 2d0b85b..74e130d 100644
--- a/cmd/koralmapper/main_test.go
+++ b/cmd/koralmapper/main_test.go
@@ -1673,28 +1673,26 @@
"match": "match:eq",
"rewrites": []any{
map[string]any{
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "foundry",
- "original": "opennlp",
+ "@type": "koral:rewrite",
+ "editor": "Koral-Mapper",
+ "original": map[string]any{
+ "@type": "koral:term",
+ "foundry": "opennlp",
+ "key": "PIDAT",
+ "layer": "p",
+ "match": "match:eq",
+ },
},
map[string]any{
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "key",
- "original": "PIDAT",
- },
- map[string]any{
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "foundry",
- "original": "stts",
- },
- map[string]any{
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "key",
- "original": "DET",
+ "@type": "koral:rewrite",
+ "editor": "Koral-Mapper",
+ "original": map[string]any{
+ "@type": "koral:term",
+ "foundry": "stts",
+ "key": "DET",
+ "layer": "p",
+ "match": "match:eq",
+ },
},
},
},
diff --git a/mapper/cascade_test.go b/mapper/cascade_test.go
index b46f3ac..cddd9a3 100644
--- a/mapper/cascade_test.go
+++ b/mapper/cascade_test.go
@@ -231,9 +231,9 @@
)
require.NoError(t, err)
- // After both steps, the term should have rewrites from both steps:
- // step 1 recorded scope=foundry original=opennlp and scope=key original=PIDAT,
- // step 2 recorded scope=foundry original=stts and scope=key original=DET.
+ // After both steps, the term should have one rewrite per step:
+ // step 1 recorded the full original term (opennlp/p/PIDAT),
+ // step 2 recorded the full original term (stts/p/DET).
expected := parseJSON(t, `{
"@type": "koral:token",
"wrap": {
@@ -246,26 +246,24 @@
{
"@type": "koral:rewrite",
"editor": "Koral-Mapper",
- "scope": "foundry",
- "original": "opennlp"
+ "original": {
+ "@type": "koral:term",
+ "foundry": "opennlp",
+ "key": "PIDAT",
+ "layer": "p",
+ "match": "match:eq"
+ }
},
{
"@type": "koral:rewrite",
"editor": "Koral-Mapper",
- "scope": "key",
- "original": "PIDAT"
- },
- {
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "foundry",
- "original": "stts"
- },
- {
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "key",
- "original": "DET"
+ "original": {
+ "@type": "koral:term",
+ "foundry": "stts",
+ "key": "DET",
+ "layer": "p",
+ "match": "match:eq"
+ }
}
]
}
@@ -312,31 +310,31 @@
)
require.NoError(t, err)
- // Step 1 rewrites (scope=foundry original=opennlp, scope=key original=PIDAT)
- // must appear on the TermGroup created by step 2, along with step 2's
- // own structural rewrite.
+ // Step 1 produced a single rewrite with the full original term (opennlp/p/PIDAT).
+ // Step 2 replaced the term with a TermGroup (structural change) and produced
+ // another single rewrite with the full original term (stts/p/DET).
+ // Both rewrites must appear on the TermGroup.
resultMap := result.(map[string]any)
wrap := resultMap["wrap"].(map[string]any)
require.Equal(t, "koral:termGroup", wrap["@type"])
rewrites := wrap["rewrites"].([]any)
- // First rewrite is from step 1 (carried forward): foundry change
+ require.Len(t, rewrites, 2)
+
+ // First rewrite is from step 1 (carried forward): full original term
rw0 := rewrites[0].(map[string]any)
- assert.Equal(t, "foundry", rw0["scope"])
- assert.Equal(t, "opennlp", rw0["original"])
+ assert.Equal(t, "Koral-Mapper", rw0["editor"])
+ original0 := rw0["original"].(map[string]any)
+ assert.Equal(t, "koral:term", original0["@type"])
+ assert.Equal(t, "PIDAT", original0["key"])
+ assert.Equal(t, "opennlp", original0["foundry"])
- // Second rewrite is from step 1 (carried forward): key change
+ // Second rewrite is from step 2: full original term
rw1 := rewrites[1].(map[string]any)
- assert.Equal(t, "key", rw1["scope"])
- assert.Equal(t, "PIDAT", rw1["original"])
-
- // Last rewrite is from step 2 (structural: original is the full term)
- rwLast := rewrites[len(rewrites)-1].(map[string]any)
- assert.Equal(t, "Koral-Mapper", rwLast["editor"])
- // Structural rewrite stores the full original node (no scope)
- original := rwLast["original"].(map[string]any)
- assert.Equal(t, "koral:term", original["@type"])
- assert.Equal(t, "DET", original["key"])
+ assert.Equal(t, "Koral-Mapper", rw1["editor"])
+ original1 := rw1["original"].(map[string]any)
+ assert.Equal(t, "koral:term", original1["@type"])
+ assert.Equal(t, "DET", original1["key"])
}
func TestCascadeResponseTwoCorpusMappings(t *testing.T) {
diff --git a/mapper/mapper_test.go b/mapper/mapper_test.go
index 4e08d14..13cb7a8 100644
--- a/mapper/mapper_test.go
+++ b/mapper/mapper_test.go
@@ -690,7 +690,7 @@
expected string
}{
{
- name: "Multi-field change: foundry + layer + key all change",
+ name: "Multi-field change: single rewrite with full original",
opts: MappingOptions{
Direction: AtoB,
AddRewrites: true,
@@ -717,27 +717,20 @@
{
"@type": "koral:rewrite",
"editor": "Koral-Mapper",
- "scope": "foundry",
- "original": "opennlp"
- },
- {
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "layer",
- "original": "p"
- },
- {
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "key",
- "original": "DET"
+ "original": {
+ "@type": "koral:term",
+ "foundry": "opennlp",
+ "key": "DET",
+ "layer": "p",
+ "match": "match:eq"
+ }
}
]
}
}`,
},
{
- name: "Reverse direction: foundry + layer + key all change back",
+ name: "Reverse direction: single rewrite with full original",
opts: MappingOptions{
Direction: BtoA,
AddRewrites: true,
@@ -764,20 +757,13 @@
{
"@type": "koral:rewrite",
"editor": "Koral-Mapper",
- "scope": "foundry",
- "original": "upos"
- },
- {
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "layer",
- "original": "pos"
- },
- {
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "key",
- "original": "PRON"
+ "original": {
+ "@type": "koral:term",
+ "foundry": "upos",
+ "key": "PRON",
+ "layer": "pos",
+ "match": "match:eq"
+ }
}
]
}
@@ -850,14 +836,13 @@
{
"@type": "koral:rewrite",
"editor": "Koral-Mapper",
- "scope": "layer",
- "original": "p"
- },
- {
- "@type": "koral:rewrite",
- "editor": "Koral-Mapper",
- "scope": "key",
- "original": "DET"
+ "original": {
+ "@type": "koral:term",
+ "foundry": "opennlp",
+ "key": "DET",
+ "layer": "p",
+ "match": "match:eq"
+ }
}
]
}
@@ -867,50 +852,40 @@
assert.Equal(t, expectedData, result)
}
-func TestBuildRewritesFieldInjection(t *testing.T) {
+func TestBuildRewritesSingleObjectRewrite(t *testing.T) {
tests := []struct {
- name string
- original *ast.Term
- new_ *ast.Term
- expectedScopes []string
- hasOriginals []bool
+ name string
+ original *ast.Term
+ new_ *ast.Term
}{
{
- name: "All fields change with originals",
- original: &ast.Term{Foundry: "a", Layer: "l1", Key: "k1", Value: "v1", Match: ast.MatchEqual},
- new_: &ast.Term{Foundry: "b", Layer: "l2", Key: "k2", Value: "v2", Match: ast.MatchEqual},
- expectedScopes: []string{"foundry", "layer", "key", "value"},
- hasOriginals: []bool{true, true, true, true},
+ name: "All fields change",
+ original: &ast.Term{Foundry: "a", Layer: "l1", Key: "k1", Value: "v1", Match: ast.MatchEqual},
+ new_: &ast.Term{Foundry: "b", Layer: "l2", Key: "k2", Value: "v2", Match: ast.MatchEqual},
},
{
- name: "Injection: empty value becomes non-empty",
- original: &ast.Term{Foundry: "a", Layer: "l", Key: "k", Match: ast.MatchEqual},
- new_: &ast.Term{Foundry: "a", Layer: "l", Key: "k", Value: "v", Match: ast.MatchEqual},
- expectedScopes: []string{"value"},
- hasOriginals: []bool{false},
+ name: "Single field injection: empty value becomes non-empty",
+ original: &ast.Term{Foundry: "a", Layer: "l", Key: "k", Match: ast.MatchEqual},
+ new_: &ast.Term{Foundry: "a", Layer: "l", Key: "k", Value: "v", Match: ast.MatchEqual},
},
{
- name: "Deletion: non-empty value becomes empty",
- original: &ast.Term{Foundry: "a", Layer: "l", Key: "k", Value: "v", Match: ast.MatchEqual},
- new_: &ast.Term{Foundry: "a", Layer: "l", Key: "k", Match: ast.MatchEqual},
- expectedScopes: []string{"value"},
- hasOriginals: []bool{true},
+ name: "Single field deletion: non-empty value becomes empty",
+ original: &ast.Term{Foundry: "a", Layer: "l", Key: "k", Value: "v", Match: ast.MatchEqual},
+ new_: &ast.Term{Foundry: "a", Layer: "l", Key: "k", Match: ast.MatchEqual},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
rewrites := buildRewrites(tt.original, tt.new_)
- require.Len(t, rewrites, len(tt.expectedScopes))
- for i, rw := range rewrites {
- assert.Equal(t, RewriteEditor, rw.Editor)
- assert.Equal(t, tt.expectedScopes[i], rw.Scope)
- if tt.hasOriginals[i] {
- assert.NotNil(t, rw.Original, "expected original for scope %s", tt.expectedScopes[i])
- } else {
- assert.Nil(t, rw.Original, "expected no original for scope %s (injection)", tt.expectedScopes[i])
- }
- }
+ require.Len(t, rewrites, 1, "one rule application should produce exactly one rewrite")
+ rw := rewrites[0]
+ assert.Equal(t, RewriteEditor, rw.Editor)
+ assert.Empty(t, rw.Scope, "object-level rewrite should have no scope")
+ assert.NotNil(t, rw.Original, "rewrite should contain the full original")
+ originalMap, ok := rw.Original.(map[string]any)
+ require.True(t, ok)
+ assert.Equal(t, "koral:term", originalMap["@type"])
})
}
}
diff --git a/mapper/query.go b/mapper/query.go
index 62022e5..95b651c 100644
--- a/mapper/query.go
+++ b/mapper/query.go
@@ -287,51 +287,15 @@
}
}
-// buildRewrites creates Rewrite entries describing what changed between
-// originalNode and newNode. For term-level changes it emits one scoped
-// rewrite per changed field so the transformation is fully reversible.
-// For structural changes it stores the full original as an object.
+// buildRewrites creates a single Rewrite entry describing what changed between
+// originalNode and newNode. One rule application on one object always produces
+// exactly one koral:rewrite with the full original serialized in `original`.
+// Rewrites from previous cascade steps are stripped from the original so the
+// serialized value only contains the node's own content.
func buildRewrites(originalNode, newNode ast.Node) []ast.Rewrite {
- if term, ok := originalNode.(*ast.Term); ok && ast.IsTermNode(newNode) && originalNode.Type() == newNode.Type() {
- newTerm := newNode.(*ast.Term)
- var rewrites []ast.Rewrite
-
- if term.Foundry != newTerm.Foundry {
- rw := ast.Rewrite{Editor: RewriteEditor, Scope: "foundry"}
- if term.Foundry != "" {
- rw.Original = term.Foundry
- }
- rewrites = append(rewrites, rw)
- }
- if term.Layer != newTerm.Layer {
- rw := ast.Rewrite{Editor: RewriteEditor, Scope: "layer"}
- if term.Layer != "" {
- rw.Original = term.Layer
- }
- rewrites = append(rewrites, rw)
- }
- if term.Key != newTerm.Key {
- rw := ast.Rewrite{Editor: RewriteEditor, Scope: "key"}
- if term.Key != "" {
- rw.Original = term.Key
- }
- rewrites = append(rewrites, rw)
- }
- if term.Value != newTerm.Value {
- rw := ast.Rewrite{Editor: RewriteEditor, Scope: "value"}
- if term.Value != "" {
- rw.Original = term.Value
- }
- rewrites = append(rewrites, rw)
- }
-
- if len(rewrites) > 0 {
- return rewrites
- }
- }
-
- // Structural change: serialize the original as the rewrite value
- originalBytes, err := parser.SerializeToJSON(originalNode)
+ clean := originalNode.Clone()
+ ast.StripRewrites(clean)
+ originalBytes, err := parser.SerializeToJSON(clean)
if err != nil {
return []ast.Rewrite{{Editor: RewriteEditor}}
}