morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #6461] [CLOSED] fix(python): keep negative-zero pyc float constants distinct #7356

New issue

Closed

opened 2026-11-01 21:47:17 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:17 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6461
Author: @Lew1sWong
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/pyc-negative-zero-floats

📝 Commits (1)

4276a47 fix(python): keep negative-zero pyc float constants distinct

📊 Changes

5 files changed (+72 additions, -6 deletions)

View changed files

📝 docs/python-pyc.md (+3 -2)
📝 src/domain/python/pythonPyc.ts (+4 -1)
📝 src/python/PythonPycParser.test.ts (+21 -0)
📝 src/python/PythonPycParser.ts (+5 -3)
📝 tests/acceptance/pythonPycCli.test.ts (+39 -0)

📄 Description

Summary

Fixes #6437. inspect-python-pyc now keeps a negative-zero float constant, and negative-zero complex parts, distinct from positive zero, instead of reporting them as an observed 0.

Problem and expected behavior

jsonFloat in src/python/PythonPycParser.ts returned finite values as JSON numbers. JSON has no negative zero, so a stored -0.0 serialized as 0. The result still claimed confidence: "observed" and complete coverage, with no limitation naming the sign loss.

Expected, per the issue and the value-preservation rule: positive and negative zero stay distinguishable in the serialized constant summary.

Change and scope

The constant summary already spells values that JSON numbers cannot carry with CPython's repr: inf, -inf and nan. Negative zero is the same case. jsonFloat now returns "-0.0" (CPython's repr(-0.0)) when Object.is(value, -0).
jsonFloat is the single owner of this conversion for float constants and both complex parts, so all three are fixed by the same change. Positive zero stays the number 0.
The output schema's float spellings and docs/python-pyc.md include -0.0.

Not included: nothing else reads these values; the parser, provider and service pass the summary through unchanged.

Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: jsonFloat in the pyc parser, where marshal floats become the JSON summary.
Contribution scope fit and linked maintainer decision: not applicable (bug fix).
CLI and MCP/tool-catalog contract: additive. The float-spelling enum used by float values and complex parts gains "-0.0". It appears only for negative-zero constants, which were previously reported as 0.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: the constant summary now preserves the observed sign of zero.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none committed. npm run docs:check passes.
Read existing files or captured data.

Evidence and regression coverage

Tests added:
- tests/acceptance/pythonPycCli.test.ts: public CLI workflow on a pyc built with the existing marshal fixture helpers. The constants are A = 0.0, B = -0.0 and C = -0j. Expects 0, "-0.0", and a complex with "-0.0" real and imaginary parts, parsed through the tool's output schema.
- src/python/PythonPycParser.test.ts: binary and marshal-version-0 text floats and complexes, including a complex whose real part is positive zero and whose imaginary part is negative zero. Also checks schema validity and a lossless JSON round trip.
Base reproduction: the issue's CPython 3.11 fixture on e32a0494d (macOS arm64) reported 0.0, -0.0 and -0j as 0, 0 and 0/0, with coverage: complete and confidence: observed. On this branch the same file reports 0, "-0.0" and "-0.0"/"-0.0", matching CPython's marshal decode.
Both new tests fail on the unmodified parser and schema (2 failed, 41 passed) and pass with the change.

User-visible CLI output (inspect-python-pyc two.pyc --json, code_objects[0].consts):

[{"kind":"float","value":0},{"kind":"float","value":"-0.0"},
 {"kind":"complex","real":"-0.0","imag":"-0.0"},{"kind":"none"}]

Remaining proof gaps: the CPython oracle check used 3.11 only; the other marshal layouts are covered by the synthesized fixtures.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Node 24.22.1, macOS 27.0 arm64, CPython 3.11.9 as the oracle. Head 4276a47c5 on 34563fcc9.

npm run check:fast — passed (typecheck, lint 0 errors)
npm run knip — passed
npm run docs:check — passed
npm run test:focused -- on every pyc test (parser, domain, service, filesystem boundary, MCP, CLI acceptance, local tool contracts, tool effects) — 9 files, 100 tests passed
npm run test:focused -- on src/contracts, tests/conformance/contracts and the MCP schema/catalog boundary tests — 59 files: 58 passed, 1 failed, 5 tests skipped.
- The failure is tests/boundary/mcp/advertisedPatternsVMode.test.ts, on a start_adb_activity number pattern that does not compile with the v flag. It fails identically on unmodified 34563fcc9 and this PR changes no pattern, so it is an upstream main breakage rather than an effect of this change.
Full npm run check:pr — not run

Compatibility, safety, and release

Breaking changes or migration steps: none. Consumers that already handle the inf/-inf/nan strings see one more spelling, only where 0 was previously wrong.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; checked against real CPython 3.11 output.
Package or release metadata impact: none.
Security, privacy, process, or containment review: static file parsing only.

Author readiness

Human owner: @Lew1sWong

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6461 **Author:** [@Lew1sWong](https://github.com/Lew1sWong) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/pyc-negative-zero-floats` --- ### 📝 Commits (1) - [`4276a47`](https://github.com/morluto/rea/commit/4276a47c56071dfa3be636357157235208bdb18d) fix(python): keep negative-zero pyc float constants distinct ### 📊 Changes **5 files changed** (+72 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `docs/python-pyc.md` (+3 -2) 📝 `src/domain/python/pythonPyc.ts` (+4 -1) 📝 `src/python/PythonPycParser.test.ts` (+21 -0) 📝 `src/python/PythonPycParser.ts` (+5 -3) 📝 `tests/acceptance/pythonPycCli.test.ts` (+39 -0) </details> ### 📄 Description ## Summary Fixes #6437. `inspect-python-pyc` now keeps a negative-zero float constant, and negative-zero complex parts, distinct from positive zero, instead of reporting them as an observed `0`. ## Problem and expected behavior `jsonFloat` in `src/python/PythonPycParser.ts` returned finite values as JSON numbers. JSON has no negative zero, so a stored `-0.0` serialized as `0`. The result still claimed `confidence: "observed"` and complete coverage, with no limitation naming the sign loss. Expected, per the issue and the value-preservation rule: positive and negative zero stay distinguishable in the serialized constant summary. ## Change and scope - The constant summary already spells values that JSON numbers cannot carry with CPython's repr: `inf`, `-inf` and `nan`. Negative zero is the same case. `jsonFloat` now returns `"-0.0"` (CPython's `repr(-0.0)`) when `Object.is(value, -0)`. - `jsonFloat` is the single owner of this conversion for float constants and both complex parts, so all three are fixed by the same change. Positive zero stays the number `0`. - The output schema's float spellings and `docs/python-pyc.md` include `-0.0`. Not included: nothing else reads these values; the parser, provider and service pass the summary through unchanged. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `jsonFloat` in the pyc parser, where marshal floats become the JSON summary. - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: additive. The float-spelling enum used by `float` values and `complex` parts gains `"-0.0"`. It appears only for negative-zero constants, which were previously reported as `0`. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: the constant summary now preserves the observed sign of zero. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none committed. `npm run docs:check` passes. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added: - `tests/acceptance/pythonPycCli.test.ts`: public CLI workflow on a pyc built with the existing marshal fixture helpers. The constants are `A = 0.0`, `B = -0.0` and `C = -0j`. Expects `0`, `"-0.0"`, and a complex with `"-0.0"` real and imaginary parts, parsed through the tool's output schema. - `src/python/PythonPycParser.test.ts`: binary and marshal-version-0 text floats and complexes, including a complex whose real part is positive zero and whose imaginary part is negative zero. Also checks schema validity and a lossless JSON round trip. - Base reproduction: the issue's CPython 3.11 fixture on `e32a0494d` (macOS arm64) reported `0.0`, `-0.0` and `-0j` as `0`, `0` and `0`/`0`, with `coverage: complete` and `confidence: observed`. On this branch the same file reports `0`, `"-0.0"` and `"-0.0"`/`"-0.0"`, matching CPython's `marshal` decode. - Both new tests fail on the unmodified parser and schema (2 failed, 41 passed) and pass with the change. - User-visible CLI output (`inspect-python-pyc two.pyc --json`, `code_objects[0].consts`): ```json [{"kind":"float","value":0},{"kind":"float","value":"-0.0"}, {"kind":"complex","real":"-0.0","imag":"-0.0"},{"kind":"none"}] ``` - Remaining proof gaps: the CPython oracle check used 3.11 only; the other marshal layouts are covered by the synthesized fixtures. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Node 24.22.1, macOS 27.0 arm64, CPython 3.11.9 as the oracle. Head `4276a47c5` on `34563fcc9`. - `npm run check:fast` — passed (typecheck, lint 0 errors) - `npm run knip` — passed - `npm run docs:check` — passed - `npm run test:focused --` on every pyc test (parser, domain, service, filesystem boundary, MCP, CLI acceptance, local tool contracts, tool effects) — 9 files, 100 tests passed - `npm run test:focused --` on `src/contracts`, `tests/conformance/contracts` and the MCP schema/catalog boundary tests — 59 files: 58 passed, 1 failed, 5 tests skipped. - The failure is `tests/boundary/mcp/advertisedPatternsVMode.test.ts`, on a `start_adb_activity` number pattern that does not compile with the `v` flag. It fails identically on unmodified `34563fcc9` and this PR changes no pattern, so it is an upstream `main` breakage rather than an effect of this change. - Full `npm run check:pr` — not run ## Compatibility, safety, and release - Breaking changes or migration steps: none. Consumers that already handle the `inf`/`-inf`/`nan` strings see one more spelling, only where `0` was previously wrong. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; checked against real CPython 3.11 output. - Package or release metadata impact: none. - Security, privacy, process, or containment review: static file parsing only. ## Author readiness Human owner: @Lew1sWong - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:17 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-05 01:47:58 +00:00

[PR #7356] [CLOSED] fix(xml): reject ambiguous namespace attribute identities #7828

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7356

No description provided.

Rows
Columns