From 1b6e92ed2def25f1026816ab79ebaff1f091629e Mon Sep 17 00:00:00 2001 From: David de Hilster Date: Sun, 18 Oct 2026 22:13:21 -0400 Subject: [PATCH 1/2] jsonwrite writes valid, separate arrays (4.2.37) jsonwrite grouped counted concepts (item1, item2) into a JSON array but wrote each separator after a child instead of before it. An array followed by a plain sibling -- a list of items, then a field -- came out with a stray "}" and no comma after the "]", so the file was not JSON. Two arrays side by side (item1 item2 thing1 thing2) merged into one, and an element with no attributes or children ended in a trailing comma. separate_con_count also kept collecting digits past a letter, so "a1b2" became array "a1" with id "12". All of this was copied on purpose from JsonKBRecurse and SeparateConCount in KBFuncs.nlp, to stay byte-identical; the matching KBFuncs fix goes to visualtext-files and its copies, so the two still agree. Every shape that was already valid JSON is written byte for byte as before. New fixture tests/json-arrays pins five shapes to expected files and round-trips each through jsonparse; the 4.2.36 engine fails all five. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/build-linux.yml | 30 ++++ .github/workflows/build-windows.yml | 30 ++++ .../tests/json-arrays/expected/A.json | 17 +++ .../tests/json-arrays/expected/B.json | 24 ++++ .../tests/json-arrays/expected/C.json | 14 ++ .../tests/json-arrays/expected/E.json | 18 +++ .../tests/json-arrays/expected/F.json | 36 +++++ .../tests/json-arrays/input/text.txt | 1 + .../tests/json-arrays/spec/analyzer.seq | 2 + .../tests/json-arrays/spec/arrays.nlp | 74 ++++++++++ lite/json.cpp | 133 +++++++++++------- nlp/main.cpp | 2 +- 12 files changed, 332 insertions(+), 49 deletions(-) create mode 100644 .github/workflows/tests/json-arrays/expected/A.json create mode 100644 .github/workflows/tests/json-arrays/expected/B.json create mode 100644 .github/workflows/tests/json-arrays/expected/C.json create mode 100644 .github/workflows/tests/json-arrays/expected/E.json create mode 100644 .github/workflows/tests/json-arrays/expected/F.json create mode 100644 .github/workflows/tests/json-arrays/input/text.txt create mode 100644 .github/workflows/tests/json-arrays/spec/analyzer.seq create mode 100644 .github/workflows/tests/json-arrays/spec/arrays.nlp diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 540fe19b..ec68dc6b 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -1505,6 +1505,36 @@ jobs: fi echo "[json] jsonwrite serializes and jsonparse round-trips" + - name: JSON arrays + shell: bash + run: | + set -eo pipefail + # jsonwrite groups counted concepts (item1, item2) into arrays. + # Until 4.2.37 an array followed by a plain sibling was not JSON, + # two arrays side by side merged, and "a1b2" counted as 12. Each + # case must match its expected file and survive a jsonparse round + # trip unchanged. + FIX=.github/workflows/tests/json-arrays + rm -rf "$FIX/logs" "$FIX/input/text.txt_log" + mkdir -p "$FIX/logs" + ./bin/nlp -ANA "$FIX" -WORK ./ "$FIX/input/text.txt" + if [ -f "$FIX/logs/make_ana.log" ]; then cat "$FIX/logs/make_ana.log"; fi + OUT="$FIX/input/text.txt_log/out.txt" + if [ ! -f "$OUT" ]; then + echo "FAIL(json arrays): no out.txt -- the analyzer did not build"; exit 1 + fi + got=$(tr -d '\r' < "$OUT") + want=$(printf '%s\n' 'A:same' 'B:same' 'C:same' 'E:same' 'F:same') + if [ "$got" != "$want" ]; then + echo "FAIL(json arrays): round trip, out.txt was:"; echo "$got"; exit 1 + fi + for c in A B C E F; do + if ! tr -d '\r' < "$FIX/input/text.txt_log/$c.json" | diff - "$FIX/expected/$c.json"; then + echo "FAIL(json arrays): case $c differs from expected/$c.json"; exit 1 + fi + done + echo "[json arrays] counted concepts become valid, separate arrays" + - name: callanalyzer runs another analyzer, loaded once if: matrix.id != 'docker-ubuntu-20.04' shell: bash diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index 280fdc7d..fbd29cee 100644 --- a/.github/workflows/build-windows.yml +++ b/.github/workflows/build-windows.yml @@ -1389,6 +1389,36 @@ jobs: fi echo "[json] jsonwrite serializes and jsonparse round-trips" + - name: JSON arrays + shell: bash + run: | + set -eo pipefail + # jsonwrite groups counted concepts (item1, item2) into arrays. + # Until 4.2.37 an array followed by a plain sibling was not JSON, + # two arrays side by side merged, and "a1b2" counted as 12. Each + # case must match its expected file and survive a jsonparse round + # trip unchanged. + FIX=.github/workflows/tests/json-arrays + rm -rf "$FIX/logs" "$FIX/input/text.txt_log" + mkdir -p "$FIX/logs" + ./bin/Release/nlp.exe -ANA "$FIX" -WORK ./ "$FIX/input/text.txt" + if [ -f "$FIX/logs/make_ana.log" ]; then cat "$FIX/logs/make_ana.log"; fi + OUT="$FIX/input/text.txt_log/out.txt" + if [ ! -f "$OUT" ]; then + echo "FAIL(json arrays): no out.txt -- the analyzer did not build"; exit 1 + fi + got=$(tr -d '\r' < "$OUT") + want=$(printf '%s\n' 'A:same' 'B:same' 'C:same' 'E:same' 'F:same') + if [ "$got" != "$want" ]; then + echo "FAIL(json arrays): round trip, out.txt was:"; echo "$got"; exit 1 + fi + for c in A B C E F; do + if ! tr -d '\r' < "$FIX/input/text.txt_log/$c.json" | diff - "$FIX/expected/$c.json"; then + echo "FAIL(json arrays): case $c differs from expected/$c.json"; exit 1 + fi + done + echo "[json arrays] counted concepts become valid, separate arrays" + - name: callanalyzer runs another analyzer, loaded once shell: bash run: | diff --git a/.github/workflows/tests/json-arrays/expected/A.json b/.github/workflows/tests/json-arrays/expected/A.json new file mode 100644 index 00000000..39564c59 --- /dev/null +++ b/.github/workflows/tests/json-arrays/expected/A.json @@ -0,0 +1,17 @@ +{ + "caseA": { + "item": [ + { + "id": "1", + "v": "item1" + }, + { + "id": "2", + "v": "item2" + } + ], + "name": { + "v": "name" + } + } +} diff --git a/.github/workflows/tests/json-arrays/expected/B.json b/.github/workflows/tests/json-arrays/expected/B.json new file mode 100644 index 00000000..c98fd702 --- /dev/null +++ b/.github/workflows/tests/json-arrays/expected/B.json @@ -0,0 +1,24 @@ +{ + "caseB": { + "item": [ + { + "id": "1", + "v": "item1" + }, + { + "id": "2", + "v": "item2" + } + ], + "thing": [ + { + "id": "1", + "v": "thing1" + }, + { + "id": "2", + "v": "thing2" + } + ] + } +} diff --git a/.github/workflows/tests/json-arrays/expected/C.json b/.github/workflows/tests/json-arrays/expected/C.json new file mode 100644 index 00000000..66a0cbef --- /dev/null +++ b/.github/workflows/tests/json-arrays/expected/C.json @@ -0,0 +1,14 @@ +{ + "caseC": { + "a1b": [ + { + "id": "1", + "v": "a1b1" + }, + { + "id": "2", + "v": "a1b2" + } + ] + } +} diff --git a/.github/workflows/tests/json-arrays/expected/E.json b/.github/workflows/tests/json-arrays/expected/E.json new file mode 100644 index 00000000..af4ed0f9 --- /dev/null +++ b/.github/workflows/tests/json-arrays/expected/E.json @@ -0,0 +1,18 @@ +{ + "caseE": { + "row": [ + { + "id": "1" + }, + { + "id": "2", + "cell": { + "v": "cell" + } + } + ], + "tail": { + "v": "tail" + } + } +} diff --git a/.github/workflows/tests/json-arrays/expected/F.json b/.github/workflows/tests/json-arrays/expected/F.json new file mode 100644 index 00000000..b963e0a0 --- /dev/null +++ b/.github/workflows/tests/json-arrays/expected/F.json @@ -0,0 +1,36 @@ +{ + "caseF": { + "person": [ + { + "id": "1", + "v": "person1", + "phone": [ + { + "id": "1", + "v": "phone1" + }, + { + "id": "2", + "v": "phone2" + } + ], + "email": { + "v": "email" + } + }, + { + "id": "2", + "v": "person2", + "phone": [ + { + "id": "1", + "v": "phone1" + } + ] + } + ], + "count": { + "v": "count" + } + } +} diff --git a/.github/workflows/tests/json-arrays/input/text.txt b/.github/workflows/tests/json-arrays/input/text.txt new file mode 100644 index 00000000..c0d34307 --- /dev/null +++ b/.github/workflows/tests/json-arrays/input/text.txt @@ -0,0 +1 @@ +The fixture builds its own knowledge base, not from this text. diff --git a/.github/workflows/tests/json-arrays/spec/analyzer.seq b/.github/workflows/tests/json-arrays/spec/analyzer.seq new file mode 100644 index 00000000..e174ff95 --- /dev/null +++ b/.github/workflows/tests/json-arrays/spec/analyzer.seq @@ -0,0 +1,2 @@ +tokenize nil # tokenize only -- fixture builds its own KB +nlp arrays # jsonwrite array grouping, round-tripped through jsonparse diff --git a/.github/workflows/tests/json-arrays/spec/arrays.nlp b/.github/workflows/tests/json-arrays/spec/arrays.nlp new file mode 100644 index 00000000..6e2ef584 --- /dev/null +++ b/.github/workflows/tests/json-arrays/spec/arrays.nlp @@ -0,0 +1,74 @@ +############################################### +# FILE: arrays.nlp +# SUBJ: Regression for how jsonwrite groups counted concepts into arrays. +# NOTE: Until 4.2.37 jsonwrite (and the JsonKB it was copied from) wrote +# the comma after each child, so an array followed by a plain +# sibling came out with a stray brace and no comma -- not JSON. +# Two arrays side by side merged into one, and a digit inside a +# name ("a1b2") was taken as part of the count. +# +# Each case is written to .json and compared in CI with +# expected/.json, then read back with jsonparse and written +# again; the second document must equal the first. +# +# Expected out.txt: +# A:same +# B:same +# C:same +# E:same +# F:same +############################################### + +@DECL +Mk(L("p"),L("name")) { + L("c") = makeconcept(L("p"),L("name")); + addstrval(L("c"),"v",L("name")); + return L("c"); +} +Check(L("label"),L("con")) { + jsonwrite(L("label") + ".json",L("con")); + L("first") = readfile(G("$apppath") + "/input/text.txt_log/" + L("label") + ".json"); + L("back") = makeconcept(findroot(),L("label") + "back"); + L("wrapped") = jsonparse(L("first"),L("back")); + jsonwrite(L("label") + "-again.json",down(L("back"))); + L("second") = readfile(G("$apppath") + "/input/text.txt_log/" + L("label") + "-again.json"); + if (L("wrapped") && L("first") == L("second")) + "out.txt" << L("label") << ":same\n"; + else + "out.txt" << L("label") << ":DIFFERENT\n"; +} +@@DECL + +@CODE +# A: an array, then a plain sibling +L("r") = makeconcept(findroot(),"caseA"); +Mk(L("r"),"item1"); Mk(L("r"),"item2"); Mk(L("r"),"name"); +Check("A",L("r")); + +# B: two arrays side by side +L("r") = makeconcept(findroot(),"caseB"); +Mk(L("r"),"item1"); Mk(L("r"),"item2"); Mk(L("r"),"thing1"); Mk(L("r"),"thing2"); +Check("B",L("r")); + +# C: a digit inside the base name +L("r") = makeconcept(findroot(),"caseC"); +Mk(L("r"),"a1b1"); Mk(L("r"),"a1b2"); +Check("C",L("r")); + +# E: array elements with no attributes, one with a child, then a plain sibling +L("r") = makeconcept(findroot(),"caseE"); +makeconcept(L("r"),"row1"); +L("x") = makeconcept(L("r"),"row2"); +Mk(L("x"),"cell"); +Mk(L("r"),"tail"); +Check("E",L("r")); + +# F: an array inside an element of an array, then a plain sibling +L("r") = makeconcept(findroot(),"caseF"); +L("p") = Mk(L("r"),"person1"); +Mk(L("p"),"phone1"); Mk(L("p"),"phone2"); Mk(L("p"),"email"); +L("p") = Mk(L("r"),"person2"); +Mk(L("p"),"phone1"); +Mk(L("r"),"count"); +Check("F",L("r")); +@@CODE diff --git a/lite/json.cpp b/lite/json.cpp index 5528c80b..7e5ba414 100644 --- a/lite/json.cpp +++ b/lite/json.cpp @@ -16,9 +16,11 @@ * into C++ is the only lever that removes it. * * WARN: The output must stay BYTE-IDENTICAL to what JsonKB produces, because -* existing analyzers' output files are checked in and compared. That -* includes deliberately reproducing a bug in KBFuncs.nlp's -* SeparateConCount -- see separate_con_count below. +* existing analyzers' output files are checked in and compared. +* JsonKB and this file were fixed together (4.2.37), so they still +* agree: an array followed by a plain sibling, two arrays side by +* side, and a digit inside a counted name -- see separate_con_count +* and json_recurse below. *******************************************************************************/ #include "StdAfx.h" @@ -92,23 +94,17 @@ return std::string((size_t)(copies * 2), ' '); * CR: 08/04/26 DD. * SUBJ: Split a counted concept name into base name and count. * RET: True if the name ends in digits; base and num are then set. -* NOTE: Mirrors SeparateConCount in KBFuncs.nlp, INCLUDING ITS BUG. +* NOTE: Mirrors SeparateConCount in KBFuncs.nlp. * -* That function scans backwards collecting digits and is meant to -* stop at the first non-digit, but its else branch is the no-op -* statement `L("stop");` where `L("stop") = 1;` was intended. So -* the scan never stops early and keeps collecting digits from -* earlier in the name: "a1b2" yields base "a1" and count "12" -* rather than base "a1b" and count "2". +* The scan stops at the first non-digit from the end, so "a1b2" is +* base "a1b" and count "2". Until 4.2.37 both this and the NLP++ +* original kept collecting digits past the letter (the NLP++ else +* branch was the no-op `L("stop");`), making "a1b2" base "a1" and +* count "12". * -* Reproduced deliberately. This decides which children become a -* JSON array, so "fixing" it here would silently change the output -* of any analyzer whose concept names contain interior digits. -* The two should be fixed together, as their own change. -* -* Also note the loop condition is `i > 0`, so index 0 is never -* examined: a name that is all digits after the first character -* still keeps its first character as the base. +* The loop condition is `i > 0`, so index 0 is never examined: a +* name that is all digits after the first character still keeps its +* first character as the base. ********************************************/ static bool separate_con_count( @@ -135,9 +131,9 @@ long len = (long) strlen(conname); // NOTE: `i > 0`, matching the NLP++ loop, so index 0 is never looked at. for (long i = len - 1; i > 0; --i) { - if (isdigit((unsigned char)conname[i])) - num.insert(num.begin(), conname[i]); // prepend, as L("c") + L("num") - // else: the NLP++ no-op. Deliberately does NOT break. + if (!isdigit((unsigned char)conname[i])) + break; + num.insert(num.begin(), conname[i]); // prepend, as L("c") + L("num") } if (num.empty()) @@ -178,6 +174,27 @@ for (CONCEPT *child = cg->Down(con); child; child = cg->Next(child)) return false; } +/******************************************** +* FN: JSON_HAS_ATTRIBUTES +* CR: 10/18/26 DD. +* SUBJ: Would json_attributes emit anything for this concept? +* NOTE: Mirrors JsonHasAttributes. An array element writes its "id" +* first and needs to know whether a comma follows it. +********************************************/ + +static bool json_has_attributes(CG *cg, CONCEPT *con) +{ +_TCHAR abuf[MAXSTR]; +for (ATTR *attrs = cg->findAttrs(con); attrs; cg->popAttr(attrs)) + { + abuf[0] = '\0'; + cg->attrName(attrs, /*UP*/ abuf, MAXSTR); + if (!is_count_con(cg, abuf, con)) + return true; + } +return false; +} + /******************************************** * FN: JSON_ATTRIBUTES * CR: 08/04/26 DD. @@ -264,10 +281,19 @@ return has_attrs; * FN: JSON_RECURSE * CR: 08/04/26 DD. * SUBJ: Emit a concept's children. -* NOTE: Mirrors JsonKBRecurse, including its "has concount" state machine: -* a run of counted siblings (date1, date2, ...) is emitted as a -* JSON array under the shared base name, each element carrying an -* "id" holding the count. +* NOTE: Mirrors JsonKBRecurse: a run of counted siblings with the same +* base name (date1, date2, ...) is emitted as a JSON array under +* that name, each element carrying an "id" holding the count. +* +* The separator is written before each sibling, once it is known +* what the sibling is. Until 4.2.37 it was written after, which +* put the comma inside an array that a plain sibling then closed +* (with a stray extra brace and no comma after the "]"), so the +* file was not JSON. A counted sibling with a different base name +* (item2 then thing1) was also added to the open array instead of +* starting its own. Every shape that was already valid JSON is +* written byte for byte as before -- including the array that +* follows a plain sibling opening on the same line as its "},". ********************************************/ static void json_recurse( @@ -279,7 +305,8 @@ static void json_recurse( { CONCEPT *con = cg->Down(parent); bool first = true; -bool has_concount = false; +bool in_array = false; +std::string array_base; _TCHAR nbuf[MAXSTR]; std::string base, num; @@ -288,25 +315,45 @@ while (con) nbuf[0] = '\0'; cg->conceptName(con, /*UP*/ nbuf); bool counted = separate_con_count(nbuf, /*UP*/ base, num); + bool continues = counted && in_array && base == array_base; + + if (!first) + { + if (continues) + os << _T(",\n"); + else if (in_array) + { + os << _T("\n") << json_indent(level + 1).c_str() << _T("],\n"); + in_array = false; + } + else if (counted) + os << _T(","); + else + os << _T(",\n"); + } if (counted) { - if (!has_concount) + if (!continues) + { os << json_indent(level + 1).c_str() << json_escape(base.c_str()).c_str() << _T(": [\n"); - else - os << _T("\n"); + in_array = true; + array_base = base; + } os << json_indent(level + 2).c_str() << _T("{\n"); os << json_indent(level + 3).c_str() << _T("\"id\": \"") - << num.c_str() << _T("\",\n"); - first = !json_attributes(cg, os, con, level + 2); - has_concount = true; + << num.c_str() << _T("\""); + if (json_has_attributes(cg, con)) + { + os << _T(",\n"); + json_attributes(cg, os, con, level + 2); + } if (cg->Down(con)) { - if (!first) - os << _T(",\n"); + os << _T(",\n"); json_recurse(cg, os, con, level + 2); } else @@ -314,35 +361,25 @@ while (con) } else { - if (has_concount) - { - os << _T("\n") << json_indent(level + 2).c_str() << _T("}"); - os << _T("\n") << json_indent(level + 1).c_str() << _T("]"); - has_concount = false; - } - if (!first) - os << _T("\n"); os << json_indent(level + 1).c_str() << json_escape(nbuf).c_str() << _T(": {\n"); - first = !json_attributes(cg, os, con, level + 1); + bool hasattrs = json_attributes(cg, os, con, level + 1); if (cg->Down(con)) { - if (!first) + if (hasattrs) os << _T(",\n"); json_recurse(cg, os, con, level + 1); } - else if (!has_concount) + else os << _T("\n") << json_indent(level + 1).c_str() << _T("}"); } con = cg->Next(con); - if (con) - os << _T(","); first = false; } -if (has_concount) +if (in_array) os << _T("\n") << json_indent(level + 1).c_str() << _T("]"); os << _T("\n") << json_indent(level).c_str() << _T("}"); } diff --git a/nlp/main.cpp b/nlp/main.cpp index aba38fa3..cd3322d7 100644 --- a/nlp/main.cpp +++ b/nlp/main.cpp @@ -15,7 +15,7 @@ All rights reserved. #include "lite/nlp_engine.h" #include "version.h" -#define NLP_ENGINE_VERSION "4.2.36" +#define NLP_ENGINE_VERSION "4.2.37" bool cmdReadArgs(int, _TCHAR *argv[], _TCHAR *&, _TCHAR *&, _TCHAR *&, _TCHAR *&, bool &, bool &, bool &, bool &, bool &, bool &, int &, bool &, bool &, _TCHAR *&); void cmdHelpargs(_TCHAR *); From bd9d38c77a32b96ef79a17e62ebf37afbdbb8f3b Mon Sep 17 00:00:00 2001 From: David de Hilster Date: Sun, 18 Oct 2026 22:36:47 -0400 Subject: [PATCH 2/2] JSON arrays CI step: skip ubuntu-20.04, ignore CR in expected files The Windows checkout converts expected/*.json to CRLF, and the step stripped CR from the output only, so every line differed. On Linux the step lacked the Non-20.04 guard every other test step has; that job builds in Docker and its binary cannot run on the host. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/build-linux.yml | 6 ++++-- .github/workflows/build-windows.yml | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index ec68dc6b..6af604cf 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -1505,7 +1505,8 @@ jobs: fi echo "[json] jsonwrite serializes and jsonparse round-trips" - - name: JSON arrays + - name: JSON arrays (Non-20.04) + if: matrix.id != 'docker-ubuntu-20.04' shell: bash run: | set -eo pipefail @@ -1529,7 +1530,8 @@ jobs: echo "FAIL(json arrays): round trip, out.txt was:"; echo "$got"; exit 1 fi for c in A B C E F; do - if ! tr -d '\r' < "$FIX/input/text.txt_log/$c.json" | diff - "$FIX/expected/$c.json"; then + # Strip CR on both sides: the Windows checkout gives expected/ CRLF. + if ! diff <(tr -d '\r' < "$FIX/input/text.txt_log/$c.json") <(tr -d '\r' < "$FIX/expected/$c.json"); then echo "FAIL(json arrays): case $c differs from expected/$c.json"; exit 1 fi done diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index fbd29cee..df2243ee 100644 --- a/.github/workflows/build-windows.yml +++ b/.github/workflows/build-windows.yml @@ -1413,7 +1413,8 @@ jobs: echo "FAIL(json arrays): round trip, out.txt was:"; echo "$got"; exit 1 fi for c in A B C E F; do - if ! tr -d '\r' < "$FIX/input/text.txt_log/$c.json" | diff - "$FIX/expected/$c.json"; then + # Strip CR on both sides: the Windows checkout gives expected/ CRLF. + if ! diff <(tr -d '\r' < "$FIX/input/text.txt_log/$c.json") <(tr -d '\r' < "$FIX/expected/$c.json"); then echo "FAIL(json arrays): case $c differs from expected/$c.json"; exit 1 fi done