Skip to content

Fix cross-runtime number spelling for integral float64 (#189, #190) - #192

Merged
Liam0205 merged 10 commits into
masterfrom
fix/189-190-number-spelling-parity
Aug 5, 2026
Merged

Fix cross-runtime number spelling for integral float64 (#189, #190)#192
Liam0205 merged 10 commits into
masterfrom
fix/189-190-number-spelling-parity

Conversation

@Liam0205

@Liam0205 Liam0205 commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Closes #189. Closes #190.

Both issues are one defect. Nightly diff-fuzz (#189, seed 1655185644 round 7005) and a case I preserved during the #187/#188 audit (#190) show the same float64 spelled two ways:

number spelling diagram

The root cause was not the formatter

My first instinct was GoFormat.formatJsonNumber. Probing it against Go across 2^53, 2^53+2, 2^62, 1e16, 1e17 and 1e20 showed it byte-for-byte correct at every magnitude — so the defect had to be upstream.

TransformByLua.fromLua narrowed any integral double to long:

if (d == Math.floor(d) && !Double.isInfinite(d) && d >= Long.MIN_VALUE && d <= Long.MAX_VALUE) {
    return (long) d;   // no 2^53 bound
}

The value therefore reached serialization as a Long and never entered the float path at all. doublelong stops round-tripping at 2^53, and (-2147483648)² is exactly 2^62.

pine-java was the only runtime doing this — Go's pool_gopher_lua returns float64(x) for every Lua number with no integer branch, and pine-cpp uses lua_tonumber — so Long was never a cross-runtime contract, just an internal detail that six assertions had frozen. Below 2^53 nothing changes, because an integral double prints without a decimal point (42.042).

What removing it exposed

Review round 1 found a blocking regression my own fix had caused. GoFormat.sprint branched on the box type: Long/Integer returned early, and only the Double path applied Go's 1e6 switch to %g. Once Lua values arrived as Double, the two sides of a comparison used different rules:

config side data side result
before Integer2000000 Long2000000 match (by accident)
after the narrowing fix Integer2000000 Double2e+06 no match

So filter_condition with value: 2000000 stopped removing a Lua-produced 2000000. Go and pine-cpp removed the items; Java kept them. The narrowing had been masking this by making both sides integral.

My first attempt at that fix was also wrong, instructively: I kept an Integer branch above 1e6, thinking the issue was precision. It is not — it is whether both sides obey one rule. Go has no integer branch here either, because every value reaching its %v came through encoding/json, which has no integer type.

No trade-off after all — review caught that too

I originally believed pine-java could not match Go on both the filter_condition path and the
transform_size count path, since Jackson's box type records where a value was parsed (config
literal → Integer, pipeline data → Double) rather than the reference runtime's static type. I
deleted sprint's integral-box branch, kept filter_condition, and recorded the count path as an
accepted regression.

PR review rejected that, correctly. The Redis paths affect data correctness — a key one runtime
writes is not read by another, orphaned keys accumulate, and member values under a stable key read
back different — so it cannot be frozen as accepted. And the remedy it pointed to is the right one:
handle box-type parity at the comparison site, not in the shared formatter.

My error was more basic than choosing the wrong side. I treated the formatter's output as the only
adjustable knob, and "these two cannot both hold" follows from that. The actual free variable is
whether the comparison normalizes its own two sides:

before after
GoFormat.sprint box-type branch deleted; everything took the float rule branch restored — mirrors Go's %v, which prints a native int plainly
FilterCondition compared raw sprint output normalizes both sides to double first, as Go's two float64 sides effectively are

All four sprint consumers — templated params, filter_condition, Redis keys, Redis member values —
now match Go. Both halves are independently gated: removing the normalization reproduces the original
divergence, and removing the box branch fails sprintPreservesIntegralBoxSpellingLikeGoDoes.

#190's second request: fuzz reproducibility

The runner now prints a paste-ready reproduction command carrying the flags that affect detection--stability-runs (without it an unstable round cannot be re-detected at all), --go-bin, --cpp-bin, --shrink — and deliberately omits --time-budget-seconds, which stops a run early and could cut off the round being chased.

Review round 4 then found that command could not reproduce anything: generation used list(set(...)), whose order varies with PYTHONHASHSEED, so --seed did not determine a run. Measured with --seed 42: different hash seeds gave different coverage; repeating a hash seed reproduced exactly. Fixed with sorted(), and I found a second site where a set was iterated while consuming rng.random() per field, shifting the whole downstream stream. A fixed seed now gives identical generation across hash seeds.

Verification

360 Java tests, 254 C++ cases, Go suite, lint, codegen-check, cross-validate 57 PASS, differential-fuzz 1000/1000. #189's original seed reruns clean: 7010 rounds, three engines, 7010 PASS / 0 FAIL where round 7005 previously diverged. New fixture 14_integral_float_above_2pow53 covers 2^62 both signs, the 2^53 boundary and its first representable step, plus controls; it and every new test were verified red against the reverted fix.

make fmt-check not run — no clang-format locally and no CI job, an existing tracked gap.

Review history worth knowing before merge

Seven blind review rounds in isolated fixed-commit snapshots. Two real code defects, both mine: the regression above (r1) and the fuzz nondeterminism (r4). The other five rounds found no code defect — each found my description of the accepted trade-off being incomplete: a false premise about Go, a divergence mislabelled as pre-existing when the range introduced it, an understated consumer set, a list naming two of three, then a fourth surface. Each time I added the missing entry and the next round found another.

That is why rounds 6 and 7 stopped rewriting prose: the consumer enumeration is derived from source by a test, which justified itself by failing on its first run and catching an error my prose had carried. Round 7 then closed the hole where a comment naming the symbol could mask a removal.

Three terminal evidence audits, all three FAIL, none on code. A missing scope addendum plus a false claim that none was needed; then a hand-written commit column omitting a third of its entries and a count corrected beside an uncorrected list; then the superseded paragraph left standing next to its replacement, asserting the number the replacement had overturned. Same mechanism every time — I changed a sentence and left the text that made it wrong. What worked, both times it was tried, was removing the human step.

Then the PR bot found the thing seven local rounds had not: the trade-off those five rounds kept
describing more precisely was not a real trade-off. Precision spent on a wrong conclusion costs more
than the conclusion did — that lesson is in the reflection.

Three gaps remain recorded with decision options rather than fixed: negative zero (reproduces at base;
Go is self-inconsistent, so the reference needs defining first), Codegen.toPythonLiteral's axis
mismatch (real in principle, unreachable today), and cross_storage_diverge incremented but never
printed or gated. The fourth — the transform_size static type — is no longer a gap; it is fixed.

Both issues are the same defect. Nightly fuzz (#189, seed 1655185644 round 7005)
and a case I preserved during the #187 audit (#190) both show go-vs-java byte
divergence on one float64:

  go   4611686018427388000
  java 4611686018427387904

Same value — (-2147483648)^2 is exactly 2^62 — spelled two ways. Go's
encoding/json emits strconv's shortest round-trip; Java emitted the exact integer.

The cause is not the formatter. GoFormat.formatJsonNumber already matches Go
bit-for-bit on every value I probed, 2^53 through 2^62 and 1e20. The divergence came
from fromLua narrowing any integral double to long, so the value reached
serialization as a Long and never went through the float path at all. That
narrowing had no upper bound, and double<->long stops round-tripping at 2^53.

pine-java was the only runtime doing this: Go's pool_gopher_lua returns float64(x)
for every Lua number with no integer branch, and pine-cpp uses lua_tonumber. So
Long was never a cross-runtime contract. Removing it makes all three agree, verified
byte-identical on the preserved case and on a new fixture.

Six existing assertions had frozen the box type rather than the contract, and they
document why this is safe: nothing below 2^53 changes, because
formatJsonNumber prints an integral double without a decimal point, so 42.0 still
serializes as 42. Updated to assert values and serialized form.

FixtureTest needed a real fix, not an expectation update. Its assertValueEquals
compared numbers numerically but fell through to String.valueOf for lists and maps,
so [10.0, 15.0] failed against the fixture literal [10, 15] on formatting alone —
and only here, because Go's fixture runner compares with fmt "%v", which prints
float64 10 as "10". It now recurses into containers, which is what it already did
for scalars.

Also addresses #190's second request: differential-fuzz now prints a REPRODUCE line
with the failing round numbers and a ready-to-paste command. Reproducing #189
required guessing, because the seed appeared only in the header and the saved
divergence directory lives in /tmp and gets reaped — that directory is the only
reason #190 could be diagnosed at all.

New fixture 14_integral_float_above_2pow53 covers the band no fixture reached:
2^62 both signs, the 2^53 boundary and its first representable step, plus a small
integer and a fraction as controls. #180's 06_number_format_regimes covers integer
expansion versus scientific notation; this is the other direction, two integer
spellings of one float64.

Verified: reran #189's exact seed for 7010 rounds with all three engines — 7010
PASS, 0 FAIL, where round 7005 previously diverged. 357 Java tests, 254 C++ cases,
Go suite, lint, codegen-check, cross-validate 57 PASS, byte-exact 13/13 both pairs,
differential-fuzz 1000/1000. Fixture and unit test both verified red against the
reverted fix.
Reflection: memory/reflections/lua-integral-double-narrowing-189-190.md

The sharpest fact from this task is not the one-line fix. The narrowing was
introduced on 2026-05-18 and issue #175, on 2026-07-23, rewrote scalar dispatch in
that same function three lines away — and left it. #175's own follow-up reflection
then recorded "all three is*() dispatch points are closed; no extra sweep needed
next time you touch this file", which is precisely the sentence that let this
survive.

So must/conventions.md gains a rule beside the existing one about measuring the
affected surface: a function is the audit unit, a dimension is not. #175's checklist
asked "is the dispatch predicate correct"; this defect was "is the conversion after
dispatch correct" — same function, same screen, different dimension, structurally
invisible to a grep for is*() calls. Recorded explicitly as a DIFFERENT failure mode
from "clean up every copy of a claim" (#183, #179): that one is one dimension across
many places, this one is one place across many dimensions.

reference/number-formatting-parity.md gains the scope statement it never had. It
covers how an already-double value becomes a string, and its unstated premise is
that the value arrives as a double. That premise is why searching it led nowhere
here: formatJsonNumber is byte-for-byte correct at every magnitude I probed, and the
divergence still happened, because the value was converted to Long upstream and
never entered the float path.

Three guide entries. Shared fixtures hide a variable — Go's runner compares with
fmt "%v" while Java's assertValueEquals fell through to String.valueOf for
containers, so [10.0, 15.0] failed against the literal [10, 15] in Java only, on
formatting rather than value; the fix belongs in the comparison, not the
expectations. Fuzz failures must carry their own reproduction command, because the
seed appeared only in the header and the saved divergence directory lives in /tmp;
#190 was diagnosable purely because a copy survived. And nothing may touch build
outputs while a long fuzz runs — I recompiled Java mid-run and got eight false
divergences from a half-written target/classes, and briefly believed I had
introduced a defect.

investigation-to-fix-testing.md gains the box-type-versus-contract rule. Six
assertions froze Long, which was never a cross-runtime contract: Go returns
float64(x) for every Lua number with no integer branch and pine-cpp uses
lua_tonumber. Freezing an internal detail inverts the signal — the correct fix reads
as a regression.

doc-gaps.md records one latent gap found by sweeping for the same pattern:
Codegen.toPythonLiteral dispatches on the value while Go's pythonLiteral dispatches
on the static type, so they diverge in principle (1e16 gives Go 1e+16 and Java the
integer form). Unreachable today — no operator spec carries a large enough
whole-number float default and codegen-check is clean — so it is recorded, not fixed.
Blind review found a blocking regression caused by the previous commit, and it is
the more interesting half of this task.

GoFormat.sprint branched on the box type: Long and Integer returned early via
Long.toString, and only the Double path applied Go's < 1e6 switch to %g. Once
fromLua stopped narrowing integral doubles, Lua-produced values arrived as Double,
so the two sides of a comparison were formatted by different rules —
filter_condition with value 2000000 stopped matching a Lua-produced 2000000,
because the config side printed "2000000" and the data side "2e+06". Go and
pine-cpp both removed the items; Java kept them. The narrowing had been masking
this by making both sides integral by accident.

My first fix was wrong in an instructive way. I kept an Integer branch above 1e6,
reasoning that an integral box has an exact decimal form a double may not. It still
diverged, because the asymmetry is not about precision — it is about the two sides
obeying different rules at all. Go has no integer branch here either, and that is
the point: every value reaching Go's %v came through encoding/json, which has no
integer type, so both sides are float64 and both take the same 1e6 switch.
Mirroring Jackson's Integer-versus-Double distinction was mirroring a distinction
the reference implementation does not have.

The regression also shows what the six reverted assertions had really been holding:
not the Long box type, which was never a contract, but the stability of sprint
output. Nothing covered that, so a new test asserts sprint is identical whether a
literal is boxed as Integer or Double, across the 1e6 boundary, and it goes red
when the box-type branch is reintroduced.

The fuzz REPRODUCE line omitted the flags that matter most for the failures needing
reproduction: without --stability-runs an unstable round prints a command that
cannot re-detect instability, since the flag defaults to 0 and disables the check
entirely; without --cpp-bin a sanitizer-specific divergence silently re-runs against
the Release build. It now carries the flags actually in effect, and deliberately
omits --time-budget-seconds, which stops a run early and could cut off the round
being chased.

FixtureTest's new Map branch dropped key-order sensitivity that String.valueOf had.
That is defensible — order is pinned directly by GoJsonKeyOrderParityTest and
cross-validate's key_order_signature rather than as a side effect of
stringification — but it was unstated, so it is now stated.

Recorded rather than fixed: the reviewer's negative-zero observation reproduces at
the base commit and Go itself is inconsistent on it, so the reference behaviour needs
defining before the other two can follow.

358 Java tests, 254 C++ cases, Go suite, lint, codegen-check, cross-validate 57
PASS, differential-fuzz 1000/1000.
Second review, one blocking finding, and it is about the comment rather than the
code: I wrote that Go has no integer branch in its %v path "because every value
reaching it came through encoding/json as a float64". That is false. Go's
transform_size writes in.ItemCount() — a native Go int — directly into a common
field, and Go's %v prints an int plainly while applying the 1e6 switch only to
float64.

The behaviour stays. The real reason the box-type branch has to go is narrower and
now stated: Java cannot reconstruct that distinction, because Jackson's box type
records where a value was parsed — a config literal becomes Integer, pipeline data
becomes Double, for the same number — not what the reference runtime considers its
static type. Branching on it therefore formats the two sides of one comparison under
different rules, which is the regression the previous commit fixed.

On the single path where Go really does hold an int, pine-cpp casts item_count() to
double and lands with pine-java against Go, so Go is the outlier and no change to
this function settles it. Recorded in doc-gaps with three decision options, along
with what I measured: below 1e6 all three agree, and a JSON request value at 1e6
fails identically everywhere because that path decodes to float64 in Go too.

The reflection was stale for the second commit — pinned file and line counts, a
pinned test count, and no mention of the box-type cause at all. The counts are gone
rather than corrected, and the second root cause is written up with the shape both
halves share: using the host language's type system as a proxy for a cross-runtime
contract. The narrowing had long standing in for "integer"; the box type stood in
for "static type". Also recorded that my first attempt at the box-type fix was wrong
for an instructive reason — I preserved an integral branch above 1e6 thinking the
issue was precision, when it was whether both sides obey one rule.

358 Java tests, 254 C++ cases, Go suite, lint, codegen-check, cross-validate 57
PASS, differential-fuzz 1000/1000.
Third review, one blocking finding: I recorded a regression this range introduced as
if it were a pre-existing gap.

At base, pine-java printed an Integer 1000000 as "1000000" and therefore AGREED with
Go's %v on the native int that transform_size writes; pine-cpp was the lone outlier
because it casts item_count() to double. Removing sprint's box-type branch flipped
the sides, so pine-java now matches pine-cpp and a transform_size -> filter_truncate
top_n: "{{n}}" pipeline errors at >= 1e6 items where Go succeeds. My doc-gaps entry
said only "audit found" and listed decision options as though the range had not
caused it.

The reviewer's sharper point is that I clearly know how to label this: the
negative-zero entry in the same file says explicitly that it predates the range and
names the base commit. That one I marked and this one I did not. Accepting a
regression is legitimate; recording it as something I did not cause is not.

Not reverted, and the reviewer argued the same after measuring: keeping the box-type
branch breaks filter_condition, and keeping it only above 1e6 reintroduces the same
asymmetry. The two paths cannot both agree with Go, because Java's box type tracks
where a value was parsed rather than its static type. filter_condition is far more
reachable than a >= 1e6 item count, so that is the side kept.

doc-gaps now states the range introduced it, with the per-commit measurements; the
GoFormat comment drops "Go is the outlier" for the flip it actually describes; the
reflection records the accepted regression rather than reporting everything green;
and integralCountAboveOneMillionUsesScientificForm pins the behaviour so the next
edit to that function cannot move it silently.

Recorded in the run evidence, not here: while measuring base behaviour I stashed a
clean tree, so nothing was stashed, and the following pop applied an unrelated
branch's pre-existing stash and produced conflicts in files this task never touches.
Recovered with reset --hard; the stash is intact. Use a separate worktree to inspect
another commit instead of stashing.

359 Java tests, 254 C++ cases, Go suite, lint, codegen-check, cross-validate 57
PASS, differential-fuzz 1000/1000.
Fourth review, first with no blocking finding. Two important ones.

The fuzz REPRODUCE line could not reproduce anything. The generator built field
lists with list(set(...)), and set iteration order over strings varies with
PYTHONHASHSEED, so --seed did not determine what got generated: with --seed 42,
different hash seeds produced different coverage, and repeating a hash seed
reproduced exactly. The whole purpose of that line is reproducing a failure without
the /tmp divergence copy, so this defeated it.

Fixed at the root with sorted() rather than by printing PYTHONHASHSEED, so the seed
alone remains sufficient. Sweeping for other hash-order dependencies found a second
site the review had not named: a loop iterating a set while calling rng.random() per
field, so hash order changed both which fields were nulled and how many draws were
consumed, shifting the entire downstream stream. A third set-derived site feeds only
any(), so it is order-independent and consumes no RNG; left alone. Verified --seed 42
now gives identical coverage across four hash seeds. One-time cost, recorded in the
code: seeds from before this commit will not reproduce.

My "the sides flipped" claim was true for exactly one source. Go reaches the
formatter with a native int only through transform_size's in.ItemCount(); every
other source of the same count passes through encoding/json and is a float64 in Go
too, so Go also prints 1e+06 and also errors. Measured a request payload
{"n":1000000} failing in all three. On those sources base pine-java was the lone
outlier and this range FIXED a pre-existing divergence — the opposite of what my
wording implied. Scoped in doc-gaps, in the GoFormat comment, and in the pinning
test's comment.

Rounds 3 and 4 failed the same way and the reflection now says so: round 3 omitted
who introduced a divergence, round 4 stated which runtime was the outlier too
broadly. Both are claims about the change rather than the change itself, which is
where every finding in this task since the first has landed.

359 Java tests, 254 C++ cases, Go suite, lint, codegen-check, cross-validate 57
PASS, differential-fuzz 1000/1000.
Fifth review, no blocking finding for the second round running. One important
finding, again about a claim rather than the code.

My scope sentence had the producer right and the consumer set wrong.
`transform_size` is genuinely the only native-int frame write in Go — verified
against every SetCommon/SetItem call site — but the resulting value reaches every
consumer of this formatter, and they fail differently. A templated param raises a
coerce error, which is loud. `filter_condition` comparing against that count diverges
SILENTLY: at 1e6 items Go keeps its items while pine-java and pine-cpp empty the
list, with no error at all. My wording said "one source" in a way that read as "one
code path".

That silent consumer behaved the same way at base, so it is pre-existing rather than
introduced by this range; the defect is the scope claim alone. Corrected in the
formatter comment, in the pinning test's comment, and in doc-gaps. The below-1e6
control still gives identical results in all three runtimes.

Separately recorded from the reviewer's incidental note: cross_storage_diverge is
incremented and then never printed, never added to failed_rounds, and never allowed
to affect the exit code, so a row-versus-column divergence flashes past on stdout
while nightly reports green. That is the mirror of what this task just fixed — we
added a REPRODUCE line because a failure could not be reproduced, and this is a
failure that does not count as one. Recorded with two options rather than gated,
because making it a gate could turn nightly red immediately and that needs its own
scheduling.

Worth stating plainly: round 1 found a real code regression, and every round since
has found a claim about the change — a false premise, a missing attribution, an
over-wide scope, and now an understated consumer set. The code has been stable since
round 1; my descriptions of it have not.

359 Java tests, 254 C++ cases, Go suite, lint, codegen-check, cross-validate 57
PASS, differential-fuzz 1000/1000.
Sixth review, no blocking finding for the third round running. One important
finding: my "every consumer" enumeration named two of three.

The third is Redis key construction, TransformRedisGet.sprintValue via
buildKeySuffix, used by both redis operators. It is silent like filter_condition,
and unlike filter_condition it is introduced by this range at the transform_size
source — Go writes wr:1000000 where this writes wr:1e+06, and base wrote
wr:1000000. It is the worst of the three because the effect leaves the process: a
key written by one runtime is not read back by another, and stale keys accumulate.
On every other source the same path is fixed by this change.

Rounds 2 through 6 have all found the same shape of defect — a claim about the
change stated more broadly than it holds. Round 5 said "one source" when several
consumers were affected; round 6 named two when there were three. Writing a sixth
careful sentence was not going to break that pattern, so the enumeration is now
derived from the source by a test that fails when a consumer is added or removed and
names the two documents to update.

It justified itself on the first run by failing: TransformRedisSet uses
GoFormat::sprint as a method reference, which my call-shaped patterns missed, and my
hand-written list had included it for the wrong reason — it reaches key building
through TransformRedisGet.buildKeySuffix, while its own direct use is a stream map
over list elements. Prose would have carried that error silently, which is precisely
the failure being fixed.

360 Java tests, 254 C++ cases, Go suite, lint, codegen-check, cross-validate 57
PASS, differential-fuzz 1000/1000.
…a 4th surface

Seventh review, no blocking finding for the fourth round running. One important, one
minor, and they are connected.

The important one is a fourth divergent surface. TransformRedisSet.toStringList maps
GoFormat::sprint over list elements, so it formats Redis member VALUES, not keys —
silent, and introduced by this range. It is worse than the key case in a way my
key-based reasoning did not cover: the key stays stable, so both runtimes read the
same key and get different values back. Wrong data rather than missing data. Measured
against a real Redis with data_type=list at 1e6 items.

The minor one is that last round's fix had the hole it was meant to close. Deriving
the consumer list from source was the right direction, but it matched raw file text,
so comments counted as code — removing a real call while adding a comment naming the
symbol kept the derived set unchanged and the test green. Comments are now stripped
first, verified by mutating FilterCondition exactly that way.

The reviewer bounded what the check does and does not catch: a new direct call, a
static-import call and a pure removal are detected; a consumer reaching sprint
through an existing helper is not. That bound stays and is stated rather than chased,
because a source-text check cannot follow call graphs and the helper is already
documented as the reach route.

Worth recording structurally: the consumer list is mechanical now, but the
failure-mode list beside it is still hand-written, and it has trailed the consumer
list by one entry in two consecutive rounds. The code comment and doc-gaps now say
that outright and tell whoever adds a consumer to add its failure mode too.

360 Java tests, 254 C++ cases, Go suite, lint, codegen-check, cross-validate 57
PASS, differential-fuzz 1000/1000.
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🔍 PR 审查

项目 结果
结论 REQUEST_CHANGES
审查范围 a9830fcabb3ead9b72fd6db9d9d50839f47a310d...6433f62ae8c6e349776cd2169f0ad05a05ccfe04

Lua integral double 的窄化修复方向合理,但当前实现同时引入了会影响 Redis 数据正确性的跨运行时回归。

阻塞问题 (1)

  • pine-java/src/main/java/page/liam/pine/GoFormat.java: 删除整数装箱分支后,transform_size 产生的原生整数在 >= 1e6 时被 Java 格式化为科学计数法,而 Go 仍按整数格式输出。该值进入 transform_redis_get/set 后,Java 会使用 wr:1e+06,Go 使用 wr:1000000;作为 Redis 成员值时还会在同一个键下写出不同内容。这是本 PR 新引入的静默数据契约破坏,会导致跨运行时读不到键、遗留无主键或读取到不同数据。代码注释已经确认这些路径可达,因此不能仅作为 accepted regression 固化。请保留 Lua double 修复,同时在具体比较路径处理 Integer/Double 对等,或显式保留 transform_size 的 Go 原生整数语义,避免改变 Redis 和模板等所有 sprint 消费者。(代码链接)
验证说明
  • 已检查完整范围 diff、相关调用方及完整工作树。
  • python3 -m py_compile scripts/differential-fuzz.pygit diff --check 通过。
  • Java 定向测试未能在当前 runner 执行:项目要求 JDK 25,而环境仅提供 JDK 17(invalid target release: 25)。

本次审查由 Codex 主链路 (gpt-5.6-sol) 完成。

PR review returned REQUEST_CHANGES on the accepted regression, and it was right on
both counts: the Redis paths affect data correctness — a key one runtime writes is
not read by another, orphaned keys accumulate, and member values under a stable key
read back different — so freezing that as accepted was wrong; and the remedy is to
handle box-type parity at the COMPARISON site rather than in the shared formatter.

My error was more basic than picking the wrong side. I treated the formatter's output
as the only adjustable knob, and from there "filter_condition and the transform_size
count path cannot both match Go" follows. The actual free variable is whether the
comparison normalizes its own two sides.

So GoFormat.sprint gets its integral-box branch back — Go's %v prints a native int
plainly at any magnitude, and transform_size writes in.ItemCount() without passing
through encoding/json, which is what Redis keys, Redis member values and templated
params all depend on. FilterCondition now normalizes both sides to double before
comparing, because Go's two sides both arrived as float64 through encoding/json, so
the asymmetry was always the comparison's to resolve.

All four sprint consumers now match Go. There is no accepted regression left.

Both halves are independently gated, verified by mutation: removing
FilterCondition's normalization reproduces the original divergence (Java keeps the
items Go removes), and removing sprint's box branch fails
sprintPreservesIntegralBoxSpellingLikeGoDoes. The two tests that had pinned the old
answer were rewritten — they had been asserting the behaviour this change reverses.

The broader lesson, now in the reflection: before claiming two constraints cannot
both be satisfied, confirm they act on the same adjustable point. Seven review rounds
went into describing that trade-off's scope, attribution and consumer list with
increasing precision, and its whole premise was false. Precision spent on a wrong
conclusion costs more than the conclusion did.

360 Java tests, 254 C++ cases, Go suite, lint, codegen-check, cross-validate 57
PASS, differential-fuzz 1000/1000.
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🔍 PR 审查

项目 结果
结论 APPROVE
审查范围 a9830fcabb3ead9b72fd6db9d9d50839f47a310d...3d60eb00173200babe16c2871b9ae6167ddaba27

未发现需要阻止合入的问题。Lua 数值统一保留为 Double,并在 filter_condition 比较点归一化两侧,既修复了大整数的跨运行时拼写差异,也保留了原生整数消费者的 Go %v 语义;相关 fixture、单元测试及 fuzz 可复现性改动覆盖了主要回归路径。

验证说明
  • git diff --check 通过。
  • scripts/differential-fuzz.py Python 编译检查通过。
  • Java 测试未能在当前 runner 执行:项目要求 Java 25,而 runner 仅提供 Java 17(invalid target release: 25)。这不影响本次静态审查结论。

本次审查由 Codex 主链路 (gpt-5.6-sol) 完成。

@Liam0205
Liam0205 merged commit 733094e into master Aug 5, 2026
23 checks passed
@Liam0205
Liam0205 deleted the fix/189-190-number-spelling-parity branch August 5, 2026 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Go vs Java number spelling diverges for integral float64 >= 2^53 (fuzz round 598) Nightly diff-fuzz: 1 failures, 0 unstable (2026-07-29)

1 participant