Skip to content

deps: bump workshop-rs to v0.6.4 and map provider-backed findings through SourceMap - #410

Merged
Teakowa merged 3 commits into
mainfrom
feat/408-map-provider-findings
Sep 25, 2026
Merged

Teakowa merged 3 commits into
mainfrom
feat/408-map-provider-findings

Conversation

@e54-bot

@e54-bot e54-bot commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Fixes #408

Summary

  • Negotiate LPP 1.4 with the OPY provider and list workshop-rs/mapped-text-v1 (then text-v1) in acceptedArtifactFormats on lpp/compile; providers without 1.4 keep the existing 1.1/1.2 unmapped session.
  • Apply the returned SourceMap to the re-parsed Program before validation and transforms, adding SourceProvenance::Mapped / Provenance::Mapped. Lint, analyze and validation diagnostics resolve to authored paths.
  • Nodes without an authored origin keep span: null. A shape mismatch yields a source-map-mismatch warning and fully unmapped findings.
  • Bump workshop-rs to v0.6.4 for the public SourceMap API.

Verification

  • cargo test --workspace, clippy, fmt pass.
  • Pinned Bastion c010e1a2 with a locally built mapping-capable opy-provider: 37 lint findings, all on valid .opy lines; --profile compat keeps the mapping.
  • Ablation (skip applying the map) makes the Bastion and mapped-attribution tests fail.
  • opy-provider 0.1.38 falls back to provider-artifact as before.

Notes

The Bastion test is gated on WRIGHT_BASTION_MAIN and WRIGHT_OPY_PROVIDER. No released opy-provider emits the mapped format yet (v0.1.55 predates the opy-rs mapping PR), so CI skips it until a release lands and the pin is bumped.

Negotiate LPP 1.4 with the OPY provider and list
workshop-rs/mapped-text-v1 in acceptedArtifactFormats on lpp/compile.
When a mapped artifact is returned, apply its SourceMap to the re-parsed
Program before validation and transforms so lint, analyze, and validation
diagnostics resolve to authored source paths. Nodes without an authored
origin keep no span; a shape mismatch falls back to unmapped findings
with a source-map-mismatch warning. Providers without LPP 1.4 keep the
existing unmapped behavior.

Bump workshop-rs to v0.6.4 for the public SourceMap API.

Fixes #408

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Request changes.

  • crates/wright-driver/src/session.rs:316 — after protocol-version-mismatch, this retries lpp/initialize on the same provider process. LPP v1 §19.2 requires the client to restart the session before negotiating a mutually supported version. Re-spawn the provider before the 1.1/1.2 fallback and cover that fallback with a protocol-level test; otherwise conforming pre-1.4 providers can fail even though #408 requires them to retain the existing unmapped path.
  • crates/wright-driver/src/session.rs:618-638 — mapped paths are only resolved for lint. analyze returns semantic_facts with raw {file,start,end} spans and never applies the loaded source-file table. #408 explicitly requires wright analyze locations (and deterministic JSON/text mapped paths). Resolve analyze spans through the mapped file table and add coverage for the analyze path.
  • crates/wright-driver/src/source_provider.rs:6-7 — Wright redeclares the canonical workshop-rs/text-v1 and workshop-rs/mapped-text-v1 identifiers even though workshop-rs exports them. Consume the owner constants (for example workshop_rs::program::{TEXT_V1, MAPPED_TEXT_V1}) instead of creating a second authority.
  • Cargo.toml:14 / commit f48594c — this advances the shipped workshop-rs pin but is classified as feat:. The repo AGENTS contract requires owner dependency-pin PRs to use the deps: Conventional Commit type so the release flow classifies the product-graph change correctly. Ensure the merge/squash commit uses deps:.

Respawn the provider process before the pre-1.4 fallback, since LPP allows one initialize per session. Resolve mapped file paths for analyze facts. Use the workshop-rs artifact format constants.

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The three code findings from the previous review are fixed, and the new CI run is fully green. Two remaining delivery/acceptance items:

  • crates/wright-driver/tests/source_provider.rs:556 — the pinned Bastion regression still exercises only lint. #408's acceptance criteria explicitly require both wright lint and wright analyze locations on the pinned Bastion revision, and the repo's real-project rule requires the real workflow in addition to synthetic coverage. Extend this gated Bastion check to run analyze and verify its non-null spans resolve to valid authored .opy paths/lines.
  • The PR title is still feat: ... while this PR advances the shipped workshop-rs pin. The repository delivery contract requires such owner dependency-pin changes to land with the deps: Conventional Commit type. Rename the PR / otherwise make the squash-merge title deps: ... so the release flow classifies it correctly.

Everything else from the previous review is resolved.

@e54-bot e54-bot changed the title feat: map provider-backed Wright findings through SourceMap deps: bump workshop-rs to v0.6.4 and map provider-backed findings through SourceMap Sep 25, 2026

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. The previous findings are resolved: the pinned Bastion regression now verifies analyze locations, and the PR is classified as deps: for the shipped workshop-rs pin update. No new actionable findings.

@Teakowa
Teakowa merged commit 7834ccc into main Sep 25, 2026
22 checks passed
@Teakowa
Teakowa deleted the feat/408-map-provider-findings branch September 25, 2026 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Map provider-backed Wright findings through SourceMap

2 participants