Skip to content

Prepare validation branch for staging (post validation) - #23

Merged
s-weigand merged 7 commits into
mainfrom
updates/prep_for_staging
Sep 13, 2026
Merged

s-weigand merged 7 commits into
mainfrom
updates/prep_for_staging

Conversation

@jsnel

@jsnel jsnel commented Sep 12, 2026

Copy link
Copy Markdown
Member

The staging branch was validated - we need to prep this repo to handel some additional input validation as well as (reference) root as defined by environment variables.

jsnel and others added 2 commits September 13, 2026 00:40
Allow callers to choose separate reference and current result directories while retaining existing defaults.

Reject unsupported validator selections before running tests.
Commits:

Bump actions/setup-python from 5 to 6
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 5 to 6.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v5...v6)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Bump actions/checkout from 4 to 6

Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 6.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v4...v6)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Bump actions/upload-artifact from 4 to 6

Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 6.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@v4...v6)

---
updated-dependencies:
- dependency-name: actions/upload-artifact
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Bump actions/download-artifact from 4 to 7

Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 4 to 7.
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v4...v7)

---
updated-dependencies:
- dependency-name: actions/download-artifact
  dependency-version: '7'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
[pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.6.0 → v6.0.0](pre-commit/pre-commit-hooks@v4.6.0...v6.0.0)
- [github.com/astral-sh/ruff-pre-commit: v0.4.8 → v0.16.6](astral-sh/ruff-pre-commit@v0.4.8...v0.16.6)
- [github.com/pre-commit/mirrors-mypy: v1.10.0 → v2.3.1](pre-commit/mirrors-mypy@v1.10.0...v2.3.1)
- [github.com/codespell-project/codespell: v2.3.0 → v2.4.3](codespell-project/codespell@v2.3.0...v2.4.3)
- [github.com/rhysd/actionlint: v1.7.1 → v1.7.12](rhysd/actionlint@v1.7.1...v1.7.12)
Copilot AI lite review requested due to automatic review settings September 12, 2026 22:44

Copilot AI 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.

🔵 Needs a closer look

Mixed approval-readiness assessments warrant final human review.

Pull request overview

Prepares the staging validation branch with configurable result paths, input validation, and updated CI tooling.

Changes:

  • Adds configurable reference and current result roots.
  • Validates action inputs and pytest paths.
  • Updates pre-commit and GitHub Actions dependencies.
File summaries
File Description
pyglotaran-examples/test_result_consistency.py Supports configurable result directories.
action.yml Adds root inputs and validator validation.
.pre-commit-config.yaml Updates pre-commit tool versions.
.github/workflows/test-pyglotaran-examples.yml Updates workflow action versions.
.github/workflows/run-tests.yml Updates checkout and Python setup actions.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI 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.

🟡 Changes recommended

The critical mypy dependency issue and moderate fallback-path regression must be addressed.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

action.yml:45

  • These environment variables are now set on every action invocation, including when the new inputs are omitted. That bypasses the existing fallback behavior in get_compare_results_path() (clone under HERE) and get_current_result_path() (use ~/pyglotaran_examples_results), so callers that previously relied on those defaults now fail with ValueError unless workspace-relative comparison-results and comparison-results-current directories already exist. Leave the variables unset when no root was supplied, or otherwise preserve the previous fallback paths.
        PYGLOTARAN_REFERENCE_ROOT: ${{ inputs.reference_root }}
        PYGLOTARAN_CURRENT_ROOT: ${{ inputs.current_root }}
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .pre-commit-config.yaml

Copilot AI 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.

🟢 Approval recommended

The changes are internally consistent, use valid dependency versions, and introduce no identified regressions.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Add an opt-in comparison mode for validating results across format versions while retaining legacy checks.

- Normalize layouts, labels, dimensions, metadata, and weights
- Apply scenario-specific fit tolerances and fail closed on invalid artifacts
- Produce JSON and Markdown reports with documented differences
- Add coverage for cross-version comparisons and regressions

@s-weigand s-weigand left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for reworking the validation and adding the ability to compare v0.7 and v0.8 results 🙏

Just one small cleanup request we discussed and making single array value extraction more robust.

Comment thread .github/workflows/run-tests.yml Outdated
Comment thread semantic/compatibility/normalize.py Outdated
Comment thread semantic/compare_results.py Outdated
Convert only single-element values to JSON-friendly scalars, while leaving larger arrays intact for fallback serialization. Add coverage for both cases

limit CI to Python 3.10

@s-weigand s-weigand left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's gooo, ship it 🚀

@s-weigand
s-weigand merged commit 7a729ca into main Sep 13, 2026
34 checks passed
@s-weigand
s-weigand deleted the updates/prep_for_staging branch September 13, 2026 22:31
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.

3 participants