fix: deprecated utcnow, broken Zhou rescaling, broken README quickstart - #10
Merged
Merged
Conversation
Three correctness fixes plus a schema-compliance gap: * io/manifest.build_manifest was calling datetime.utcnow(), deprecated in Python 3.12 and slated for removal in 3.14. Replaced with a timezone-aware now(UTC) that keeps the same "...Z" string format so run-id hashes stay stable. * _zhou.mixing_time_upper_bound rescaling was wrong: it contained a literal no-op (epsilon / epsilon) and dropped the 1/gap factor, so any eps != result.epsilon returned an incorrect mixing time. ZhouPredictorResult now carries the gap and Petermann factor used to build it, and the rescale formula is the analytic correction t_upper(eps_new) = t_upper(eps_old) + log(eps_old/eps_new)/Delta. * README quickstart referenced symbols that do not exist (Chain1D, XXZ, boundary_dephasing, layers="all", tau_eff, ci95). Replaced with code that actually runs against the public API and produces a meaningful result. Schema gap: * MANIFEST_SCHEMA.json requires schema_version, taxonomy_version, and diagnostic_schema_version fields, but no code path emitted them. Added io.manifest_payload (schema-compliant projection of a report), io.dump_manifest (writes that payload as JSON), and io.validate_manifest (built-in subset check + jsonschema validation when the library is installed). Tests: +10 new (152 pass, 3 skip for missing QuTiP, ruff clean).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66645be61d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+187
to
+188
| schema_path = Path(__file__).resolve().parents[3] / "MANIFEST_SCHEMA.json" | ||
| if schema_path.is_file(): |
There was a problem hiding this comment.
validate_manifest looks for MANIFEST_SCHEMA.json at Path(__file__).resolve().parents[3], which works in this repo checkout but points outside the package in a normal site-packages install; in that environment schema_path.is_file() is false and the jsonschema validation branch is skipped even when jsonschema is installed. That means payloads that violate schema-only constraints (for example additionalProperties: false) can be accepted in production installs.
Useful? React with 👍 / 👎.
4 tasks
marcohost33-maker
added a commit
that referenced
this pull request
May 28, 2026
…or (#20) Three best-practice hardenings on top of the merged PR #10: 1. Move src/liouscope/MANIFEST_SCHEMA.json so it actually ships in the wheel. pyproject.toml declared the file under [package-data], but the schema lived at the repo root, so any wheel install would miss it. Verified by building the wheel and unzipping its contents. 2. Load the schema via importlib.resources rather than a hard-coded parents[3] / "MANIFEST_SCHEMA.json" relative path. The relative path only worked under editable installs; importlib.resources is the canonical lookup that works under editable, wheel, and zipfile installs alike. 3. validate_manifest now uses a cached Draft202012Validator (the schema declares draft/2020-12). The python-jsonschema docs explicitly recommend instantiating a specific draft validator once and reusing it rather than calling the autodetecting jsonschema.validate wrapper per call. The validator is memoised with functools.lru_cache. Plus a small timestamp tightening: * _utc_now_iso() now uses the idiomatic isoformat(timespec="microseconds").replace("+00:00", "Z") pattern. Pinning timespec guarantees a fixed-width 27-character string on every call -- without it, datetimes that happen to land on a 0-microsecond boundary would lose the fractional component and produce shorter timestamps, which is a determinism hazard for manifest hashes. Tests: +4 new (156 pass, 3 QuTiP skips). Wheel rebuild confirms the schema is included as 'liouscope/MANIFEST_SCHEMA.json'. Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three correctness fixes plus a manifest schema-compliance gap surfaced by a careful audit of the v0.2.x codebase.
Bug fixes
io/manifest.build_manifestwas callingdatetime.datetime.utcnow(). That call is deprecated in Python 3.12 and slated for removal in 3.14;pyproject.tomldeclares 3.10–3.13 support, so this would silently break on 3.14. Replaced with a timezone-awaredatetime.now(UTC)while keeping the canonical...Zsuffix so run-id hashes stay byte-stable._zhou.mixing_time_upper_boundrescaling was incorrect: it contained a literal no-op (result.epsilon / result.epsilon) and dropped the1/Δfactor, so anyepsother than the originally requested one returned the wrong mixing time.ZhouPredictorResultnow carries the spectral gap and Petermann factor used to build it, and rescaling uses the analytic correctiont_upper(eps_new) = t_upper(eps_old) + log(eps_old/eps_new)/Δ. Verified against a fresh recomputation.README quickstart referenced symbols that do not exist in the public API (
Chain1D,XXZ,boundary_dephasing,layers="all",tau_eff,ci95). It would not run. Replaced with code that actually executes against the real API (build_liouvillian,diagnose(L, rho_initial=...),report.relaxation.beta_D,report.relaxation.bca_ci_beta) and produces a meaningful result.Schema-compliance gap
MANIFEST_SCHEMA.json(v1.2.0) requiresschema_version,taxonomy_version, anddiagnostic_schema_versionat the top level, but no code path emitted them.dump_reportwrites the full nestedDiagnosticReport, which is a different artefact. Added:liouscope.io.manifest_payload(report)— schema-compliant projection.liouscope.io.dump_manifest(report, path)— writes that payload as JSON.liouscope.io.validate_manifest(payload)— built-in subset check (always runs) plusjsonschemavalidation againstMANIFEST_SCHEMA.jsonwhen the library is installed.Test plan
pytest -q— 152 pass, 3 skip (QuTiP not installed). +10 new tests.ruff check src tests— clean.mypy src/liouscope— no new errors (the pre-existing numpy stub findings are environment-only and unchanged).a_class+ BCa CI.examples/quickstart.py,examples/qutrit_v1.py,examples/jc_ep_sweep.py— all run.manifest_payloadround-trips throughvalidate_manifestwithjsonschemainstalled.https://claude.ai/code/session_01ST2XjFZqnv7AhseHqEEx3B
Generated by Claude Code