Skip to content

Release v1.19.0: multiplier tables without LM-79, fixed-axes applicability and the photometry format column - #72

Open
foadshafighi wants to merge 6 commits into
mainfrom
release/v1.19.0
Open

foadshafighi wants to merge 6 commits into
mainfrom
release/v1.19.0

Conversation

@foadshafighi

Copy link
Copy Markdown
Contributor

Summary

Release 1.19.0 changes three workbook converter rules:

  • CCT multiplier tables convert without an LM-79 anchor. The baseline entry copies the headline value type and provenance; other entries are rated, scaled from the datasheet, with extension_method: cct_multiplier and no base attestation. Anchored entries retain their existing shape.
  • Fixed-axes records (Patterns A and C) retain the applicability data authored on the records row or excluded-combinations sheet.
  • Source rows read optional photometry_format, restricted to IES, LDT and TM-33 files and embedded taxonomy tokens.

Measurement SHA: 714a712a9d1ae082a9a6ef336b17fcaafe8d12dc.

Test plan

  • Full row gate passed before each planned row commit and the documentation-fix commit: gofmt, go vet ./..., clean format check, go test -race ./..., CLI build, all nine example validations, all nine build-index --check runs, and python3 tools/schema-drift-guard.py (all 310 references resolve).
  • sh tools/validator/check-spec-version.sh 1.19.0 passed. BuilderVersion remains 0.11.0.
  • Nine published examples remain byte-identical, with unchanged grades: five standard, three core and one incomplete. Schema, taxonomy, completeness and index source trees, all goldens and the 71-row gating table are unchanged.
  • CSV/XLSX tests cover valid formats, blank formats, both format refusals, fixed-only A/C blocks, excluded-row ordering and array values, applicability refusals, unanchored and ambiguous-anchor CCT shapes, anchored compatibility and provenance-copy isolation.
  • Three CLI fixture probes convert and validate with exit 0: CCT rows are 3000 K = 1178 lm rated/scaled with no base reference and 4000 K = 1240 lm repeating the rated headline provenance; the fixed-only record carries ORB-1200-40-{FINISH}, three fixed axes, estimate 1 and one excluded combination with no covered axes; the IES source row carries photometry_format: lm63_2002 and its existing reference.
  • Nineteen CLI refusal probes exit 1: wrong file type and unknown format token; missing flux, missing baseline-axis value and baseline absent from the multiplier table; malformed fixed-axis JSON, invalid integer estimate, non-string fixed axis, negative estimate, missing excluded axes, invalid excluded-axis JSON and missing excluded reason on each of A and C.
  • Native LaunchPad claims and document-truth review completed over the full release diff, followed by a review of only the three wording fixes by the two affected specialists. No unresolved finding remains on changed lines.

Callouts for reviewers

  1. Applicability cells on A/C records and format cells were previously ignored. Malformed or schema-invalid applicability values and invalid format cells now refuse conversion. Blank version cells default to 1.19.0; supported explicitly authored versions remain supported. Older converters ignore the new format column.
  2. No schema, taxonomy, grading, index projection, published example or golden changes. Length-scaled rows still require an LM-79 anchor. Fixtures use synthetic values and public testdata bundles.
  3. Local review corrected three wording claims. Two pre-existing P1 statements in docs/authoring-patterns.md (the factorized count at line 37 and the all-rated entry description at line 299) are recorded as carried forward under the unchanged-line boundary. The claims reviewer could not independently rerun executables because macOS denied sandbox-exec; the implementation row gates and CLI measurements above were run. Local secret-pattern and harness-context files were absent. Specialist tool restrictions are advisory; no resolver entries were skipped.

@foadshafighi
foadshafighi requested a review from a team as a code owner October 18, 2026 06:47
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T07:02:55.414373Z 4514070 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown

Codex Automated Code Review

Code Review Summary

PR: Release 1.19.0: unanchored CCT multipliers, applicability for fixed-axis records, and workbook photometry formats. Reviewed all 21 changed files.

P0 - Critical Issues (Must Fix)

None found.

P1 - High Priority Issues (Should Fix)

  • Generated CCT values bypass the derived-value provenance rule. tools/validator/internal/sheet/applicability.go:293 emits value_type: rated, method: scaled values without base_attestation_ref whenever no eligible LM-79 anchor exists. This contradicts the required provenance policy and the unchanged ProvenanceMethod definition in schema/taxonomy.schema.json:1188, which describes scaling from a base measurement. Authored columns still enforce that contract through derivedBaseMethods, so equivalent authored and generated quantities receive inconsistent treatment. Schema validation misses this because the reference is not structurally required. Preserve the anchor requirement for scaled values, or represent calculations from an unmeasured headline using an appropriate method such as computed, with consistent documentation and regression tests. The exception added at docs/authoring-patterns.md:301 should be corrected accordingly.

P2 - Medium Priority Issues (Consider Fixing)

None found.

P3 - Low Priority Issues (Optional)

None found.

Positive Observations

  • CSV and XLSX regression coverage checks equivalent output, refusals, and preserved anchored behavior.
  • Photometry-format tokens come from the embedded taxonomy; a parity test checks eligible file types against the schema.
  • Unanchored baseline provenance is copied without aliasing the headline map.
  • The schema drift guard passed: all 310 $ref pointers resolve. Diff whitespace checks passed.

Go tests could not run because the read-only environment prevented creation of the Go build cache.


Automated review by OpenAI Codex

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 714a712a9d

ℹ️ 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 thread tools/validator/internal/sheet/convert.go
@greptile-apps

greptile-apps Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new blocking issue was found.

Summary

Release 1.19.0 allows CCT multiplier tables without an LM-79 anchor, keeps authored applicability on fixed-axes records, and reads optional photometry_format cells.

  • CCT tables now convert without an LM-79 anchor.
  • Fixed-axes records now keep the applicability details authors enter.
  • Source rows can name the photometry file format.
  • New records now use ULC specification version 1.19.0.

Reviews (2) · Last reviewed commit: "Reject photometry formats that contradic..." · Reviewed by Greptile

@foadshafighi

Copy link
Copy Markdown
Contributor Author

REBUTTED: the generated-CCT provenance finding asks to overturn the explicitly accepted A1 design. The owner has ruled to retain it at 714a712; no code change is requested by that ruling.

Provenance.required contains only source and method; base_attestation_ref is optional. Schema validation is applying that contract. The taxonomy describes scaled as scaling from a base measurement and gives CCT multiplier arithmetic as its example. This release deliberately permits that arithmetic to use an unanchored headline, with the departure disclosed: the baseline entry repeats the headline's own value type and provenance, the changelog says the scaled rows rest on the headline flux rather than a base measurement, and the authoring guide states the no-anchor rule. A rated headline is therefore not presented as a measured baseline.

Using computed would split the same CCT multiplier arithmetic across two method tokens according to whether an eligible LM-79 anchor exists. Plan A1 considered and rejected that alternative, and the owner accepted the choice. Authored columns retain their base-reference requirement; these generated rows are the deliberate exception for a record without an eligible anchor. Refusing that record's multiplier table is the converter defect this release removes.

The owner also fixed the 1.19.0 boundary to exclude schema or taxonomy changes. Anchored rows retain their existing shape, while the unanchored shape is covered by CSV/XLSX, schema-validity and provenance-copy tests. The full row gate and all three required CI checks passed at this head.

@github-actions

Copy link
Copy Markdown

Codex Automated Code Review

Code Review Summary

PR: Release 1.19.0: allow unanchored CCT multiplier conversion, preserve fixed-axis applicability, and support workbook photometry formats.

P0 - Critical Issues (Must Fix)

None found.

P1 - High Priority Issues (Should Fix)

  • Unanchored CCT rows violate the derived-value provenance contract — tools/validator/internal/sheet/applicability.go:291. The new branch emits rated quantities with method: scaled and extension_method: cct_multiplier but no base_attestation_ref. This contradicts the required provenance rule and the unchanged taxonomy’s definition of scaled as scaling from a base measurement. JSON Schema validation passes because it does not structurally enforce this requirement; the new tests therefore accept semantically inconsistent records. For calculations from a rated headline, use a provenance method that permits that derivation, with consistent documentation and tests, or retain the requirement for a measured base attestation.

P2 - Medium Priority Issues (Consider Fixing)

None found.

P3 - Low Priority Issues (Optional)

None found.

Positive Observations

  • Photometry formats are checked against the embedded taxonomy and matched to their source-file type.
  • Tests cover CSV/XLSX parity and malformed fixed-axis applicability.
  • All 310 schema $ref pointers resolve; the diff passes whitespace checks.

Reviewed all 21 changed files. Go tests could not run because the read-only environment prevents creation of the Go build cache.


Automated review by OpenAI Codex

@foadshafighi

Copy link
Copy Markdown
Contributor Author

REBUTTED: the A1 finding on the 4514070 head repeats the design request already decided by the owner. Ruling 1 directs retaining the unanchored scaled shape, and ruling 2 expressly leaves that rebuttal in force.

The earlier response explains the accepted departure. Provenance.required contains only source and method; the base reference is optional. The baseline repeats the headline's own type and provenance, and the changelog and authoring guide disclose that the unanchored rows derive from the headline flux. Splitting this same multiplier arithmetic into another method token was considered and rejected. Authored columns retain their reference rule; generated rows are the deliberate exception.

Commit 4514070 fixes format/type pairing under ruling 2 and does not change A1. The full row gate and all three required CI checks pass on this head. No further code change is made for this repeated, owner-ruled design request.

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.

1 participant