Skip to content

Commit 39ef8d3

Browse files
committed
fix(ci): remove a gate that could not fail on the bug but could on correct output
Fourteenth review: APPROVE, 0 blocking and 0 important for the second round running. Both minors are claims of mine that do not survive checking. The buffer note I added last round said the two thread_local StringBuffers must stay separate or the value would overwrite the key. That is wrong: RawValue copies into the output stream before returning, so they are never live at the same time. The reviewer merged them and got byte-identical output across 20,000 random nested documents on both the compact and pretty paths. The note now says the duplication is removable and gives the real (weaker) reason to keep it. Gate 14d was worse than useless and is gone. Its exponent check read repr() of the parsed float, and repr(1.23457e+06) is "1234570.0" — no "e" — so that branch could never fire. Its "exactly six significant digits" check would have failed on perfectly correct output above one second: 1000.01, 1000.02, and 448 more within the first five seconds. A check that cannot detect the defect but can fail on correct output is worse than no check at all. I tried a third framing before removing it — compare digit CAPACITY across engines, which is timing-independent and sound in principle — and it cannot discriminate either, because the available bench operator peaks near 4 ms, four significant digits, where %g and shortest-round-trip are identical. Reaching six digits needs a deliberately multi-second operator. So the honest outcome is recorded rather than papered over: all three attempts and why each failed are written where test 14d used to be, so nobody spends a fourth, and the property stays pinned by test_json.cpp's "trace duration magnitudes match Go" at the magnitudes no channel here can reach. Not every property can be gated cross-engine; saying so beats leaving a check that is permanently green or that cries wolf. 348 Java tests, 250 C++ cases, lint, codegen-check, cross-validate 55/55, differential-fuzz 1000/1000.
1 parent 352bacf commit 39ef8d3

3 files changed

Lines changed: 39 additions & 78 deletions

File tree

llmdoc/reference/json-key-order-parity.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,13 @@ RapidJSON 的 `Key()`,而 `Key()` 的转义表**是有** `b`/`f` 的——所
109109
故意跑过 1 秒的算子,对校验套件太慢。改由 `test_json.cpp`
110110
"trace duration magnitudes match Go" 直接钉格式化函数本身。
111111

112+
**这条属性尝试过三种通道级检查,全部失败,脚本里记了原因以免第四次尝试**:比数值不行(是计时);
113+
比形状不行(指数分支读的是 `repr()` 解析后的 float,`repr(1.23457e+06)``1234570.0` 不含 `e`
114+
永远不触发;而「恰好 6 位有效数字」分支会对**正确**输出误报——1 秒到 5 秒之间就有 450 个这样的值);
115+
比跨引擎的「有效数字容量」原理上成立,但手头最慢的 bench 算子只到 4 ms 即 4 位有效数字,
116+
`%g` 与最短往返在那个量级根本无从区分。**结论:不是所有属性都能在跨引擎通道上钉住,
117+
承认这一点比留一个恒绿或会误报的检查更好。**
118+
112119
纪律:**同一个字符串属性(转义、排序、数字拼写)在一个运行时里出现第二份实现时,第一反应应该是
113120
合并,而不是把规则抄一遍。** 抄一遍等于把「两处必须同步」这个约束交给未来的人记住,而本任务连续
114121
两轮证明了没人记得住。

pine-cpp/src/config/json_writer.hpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,16 @@ inline void write_go_string(rapidjson::StringBuffer& sb, const std::string& s) {
147147
// NOTE: write_json_value's string branch below repeats this same three-line
148148
// shape (clear a thread_local buffer, write_go_string into it, emit as
149149
// kStringType). That is boilerplate duplication, not a second copy of the
150-
// escaping RULES — those live only in write_go_string. The two buffers must stay
151-
// separate: a key and its value are both live within one write_json_value call,
152-
// so sharing one buffer would have the value overwrite the key.
150+
// escaping RULES — those live only in write_go_string.
151+
//
152+
// An earlier version of this note claimed the two buffers MUST stay separate
153+
// because a key and its value are both live in one write_json_value call. That
154+
// is wrong: RawValue copies into the output stream before returning, so the two
155+
// are never live at once, and merging them was measured to leave 20,000 random
156+
// nested documents byte-identical on both the compact and pretty paths. The
157+
// duplication is therefore removable, not load-bearing — the reason to leave it
158+
// is only that a shared buffer makes the lifetime harder to reason about, not
159+
// that sharing breaks.
153160
template <typename Writer>
154161
inline void write_go_key(Writer& w, const std::string& key) {
155162
thread_local rapidjson::StringBuffer key_buf;

scripts/cross-validate/06-server-http.sh

Lines changed: 22 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -715,82 +715,29 @@ print(json.dumps([shape(x) for x in snaps]))"
715715
fail "server HTTP: trace input_snapshot key order divergence (Go=$snap_go, Java=$snap_java)"
716716
fi
717717

718-
# Test 14d: trace duration_ms NUMBER FORMAT. The value is timing and therefore
719-
# not comparable, but its spelling is: Go marshals float64 with
720-
# shortest-round-trip, so it never uses exponent notation in this range and
721-
# never truncates to 6 significant digits. snprintf("%g") does both — 1234.567
722-
# became 1234.57 and 1234567 became 1.23457e+06 — and no channel could see it,
723-
# because every one of them strips duration_ms as timing. This checks the shape
724-
# instead of the number, which is deterministic.
725-
srv_total=$((srv_total + 1))
726-
dur_shape_cmd="import json, re, sys
727-
d = json.loads(sys.stdin.read())
728-
bad = []
729-
for t in d.get('trace', []):
730-
raw = t.get('duration_ms')
731-
txt = repr(raw)
732-
if 'e' in txt.lower():
733-
bad.append('exponent notation: ' + txt)
734-
digits = re.sub(r'[^0-9]', '', txt).lstrip('0')
735-
if len(digits) == 6 and float(raw) >= 1000:
736-
bad.append('suspiciously exactly 6 significant digits: ' + txt)
737-
print('ok' if not bad else '; '.join(bad))"
738-
# Uses its own config with transform_bench_cpu so the duration is non-trivial.
718+
# Test 14d intentionally does not exist.
739719
#
740-
# Honest limit of this check: duration_ms has microsecond resolution (at most
741-
# three decimals), so "%g" and shortest-round-trip only disagree once the value
742-
# exceeds 1000 ms. A 400k-iteration bench operator measures about 4 ms, so this
743-
# check confirms the shape is exponent-free and untruncated but does NOT go red
744-
# against snprintf("%g") — verified by mutation. Making it discriminate would
745-
# need a deliberately >1s operator, which is too slow for this suite. The
746-
# formatter itself is pinned by test_json.cpp's
747-
# "trace duration magnitudes match Go" case instead.
748-
DUR_CONFIG="$WORK_DIR/dur_config.json"
749-
python3 -c "
750-
import json
751-
cfg = {'pipeline_config': {'operators': {'slow': {'type_name': 'transform_bench_cpu',
752-
'iterations': 400000, 'debug': True,
753-
chr(36)+'metadata': {'item_input': [], 'item_output': []}}},
754-
'pipeline_map': {'s': {'pipeline': ['slow']}}},
755-
'pipeline_group': {'main': {'pipeline': ['s']}}}
756-
with open('$DUR_CONFIG', 'w') as f:
757-
json.dump(cfg, f)
758-
"
759-
DUR_GO_PORT=18031
760-
DUR_JAVA_PORT=18032
761-
DUR_CPP_PORT=18033
762-
"$WORK_DIR/pineapple-server" -config "$DUR_CONFIG" -addr ":$DUR_GO_PORT" >/dev/null 2>&1 &
763-
dur_go_pid=$!
764-
java -cp "$JAVA_CP" -Dpine.config="$DUR_CONFIG" -Dpine.port=$DUR_JAVA_PORT page.liam.pine.PineServer >/dev/null 2>&1 &
765-
dur_java_pid=$!
766-
dur_cpp_pid=""
767-
if [[ -n "${CPP_SERVER:-}" ]]; then
768-
"$CPP_SERVER" -config "$DUR_CONFIG" -addr ":$DUR_CPP_PORT" >/dev/null 2>&1 &
769-
dur_cpp_pid=$!
770-
fi
771-
srv_ready $DUR_GO_PORT || fail "server HTTP: 14d Go server not ready"
772-
srv_ready $DUR_JAVA_PORT || fail "server HTTP: 14d Java server not ready"
773-
DUR_REQ='{"common":{"_return_trace":true},"items":[{"id":"a"}]}'
774-
go_dur=$(curl -s -X POST -H "Content-Type: application/json" -d "$DUR_REQ" "http://localhost:$DUR_GO_PORT/execute" | python3 -c "$dur_shape_cmd" || echo parse_error)
775-
java_dur=$(curl -s -X POST -H "Content-Type: application/json" -d "$DUR_REQ" "http://localhost:$DUR_JAVA_PORT/execute" | python3 -c "$dur_shape_cmd" || echo parse_error)
776-
if [[ "$go_dur" == "ok" && "$java_dur" == "ok" ]]; then
777-
srv_pass=$((srv_pass + 1))
778-
echo " [14d] POST /execute (trace) → duration_ms number format Go and Java both shortest-round-trip"
779-
else
780-
fail "server HTTP: duration_ms number format (Go=$go_dur, Java=$java_dur)"
781-
fi
782-
if [[ -n "${CPP_SERVER:-}" ]] && $cpp_srv_ready; then
783-
cpp_srv_total=$((cpp_srv_total + 1))
784-
cpp_dur=$(curl -s -X POST -H "Content-Type: application/json" -d "$DUR_REQ" "http://localhost:$DUR_CPP_PORT/execute" | python3 -c "$dur_shape_cmd" || echo parse_error)
785-
if [[ "$cpp_dur" == "ok" ]]; then
786-
cpp_srv_pass=$((cpp_srv_pass + 1))
787-
echo " [14d] POST /execute (trace) → duration_ms number format Go vs C++ match"
788-
else
789-
fail "server HTTP: duration_ms number format C++ (=$cpp_dur)"
790-
fi
791-
fi
792-
kill $dur_go_pid $dur_java_pid ${dur_cpp_pid:-} 2>/dev/null || true
793-
wait $dur_go_pid $dur_java_pid ${dur_cpp_pid:-} 2>/dev/null || true
720+
# trace[].duration_ms goes through go_format_json_number so its spelling matches
721+
# Go's shortest-round-trip rather than snprintf("%g")'s six significant digits.
722+
# Three attempts were made to gate that here, and all three failed for the same
723+
# reason, which is worth recording so nobody spends a fourth:
724+
#
725+
# 1. Compare the value: impossible, it is timing.
726+
# 2. Compare its shape (exponent present, digit count): the exponent branch
727+
# could never fire, because it inspected repr() of the PARSED float and
728+
# repr(1.23457e+06) is "1234570.0"; and the digit-count branch would have
729+
# failed on CORRECT output above one second (1000.01, 1000.02, and 448 more
730+
# in the first five seconds).
731+
# 3. Compare digit CAPACITY across engines: sound in principle, but the
732+
# available bench operator peaks around 4 ms, i.e. four significant digits,
733+
# so %g and shortest-round-trip are indistinguishable there. Reaching six
734+
# digits needs a deliberately >1s operator, too slow for this suite.
735+
#
736+
# duration_ms carries at most three decimals (duration_us / 1000.0), so the
737+
# divergence only appears once the integer part reaches four digits. The
738+
# formatter is therefore pinned directly by test_json.cpp's "trace duration
739+
# magnitudes match Go", which asserts 1234.567, 1000.001 and 1234567 — the
740+
# magnitudes this channel cannot reach.
794741

795742
# Test 15: Content-Type header parity across endpoints
796743
srv_total=$((srv_total + 1))

0 commit comments

Comments
 (0)