Skip to content

feat(provider): emit workshop-rs/mapped-text-v1 from opy-provider - #368

Merged
Teakowa merged 2 commits into
mainfrom
feat/362-mapped-text
Sep 24, 2026
Merged

Teakowa merged 2 commits into
mainfrom
feat/362-mapped-text

Conversation

@e54-bot

@e54-bot e54-bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #362

Summary

  • opy-provider serves LPP 1.3 and 1.4. In a 1.4 session lpp/compile honors acceptedArtifactFormats and returns the first listed format it can produce (workshop-rs/mapped-text-v1 or workshop-rs/text-v1). Earlier sessions reject the field with -32602; requests without it are unchanged.
  • The mapping is the workshop-rs SourceMap, checked against the re-parsed Workshop text; file table entries are rewritten to document URIs.
  • Macro-expanded code maps to the invocation site via a separate lowering used only for the mapped artifact. Diagnostics and normal lowering keep their spans.
  • The mapped text is returned beside CompileReport by a new method; no public struct changes.

Verification

  • cargo test --workspace, cargo fmt, cargo clippy pass.
  • Pinned OWBastion/Bastion c010e1a passes via OPY_BASTION_MAIN=<src/main.opy> cargo test -p opy-provider pinned_bastion (skipped when unset).

Notes

  • The lexer counts a tab as 4 columns; unchanged here, affects diagnostics too.

Serve LPP 1.3 and 1.4. In a 1.4 session lpp/compile honors
acceptedArtifactFormats and returns the first listed format it can
produce, mapped-text-v1 or text-v1; sessions before 1.4 reject the field
with -32602 and requests without it keep the text-v1 output.

The mapping is the workshop-rs SourceMap extracted from the lowered
program, proven against the re-parsed Workshop text, with file table
entries rewritten to document URIs. Macro-expanded code now maps to the
invocation site instead of the macro definition.

Fixes #362

@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.

Three blocking findings:

  1. crates/opy-provider/src/main.rs — unsupported artifact negotiation is only refused while building a successful artifact. If acceptedArtifactFormats contains no producible format and compilation fails, artifact_json is never called, so the request returns ordinary diagnostics instead of compile.artifactFormatUnsupported. This contradicts the PR's documented 1.4 behavior and makes negotiation depend on source validity. Reject the unsupported selection before loading/compiling, and cover the failing-source case.

  2. crates/opy-rs/src/compiler/backend.rs — relocating every span in macro-expanded HIR changes the shared diagnostic spans, not just the new Workshop source map. #362 explicitly lists changes to OPY diagnostics as a non-goal, while this PR states that diagnostics inside macro expansions move to the invocation site. Keep the invocation-site policy confined to mapped-artifact attribution so existing diagnostic behavior remains unchanged.

  3. crates/opy-rs/src/compiler/mod.rs — adding public mapped: Option<MappedText> to exhaustive public CompileResult is a Rust API break (the crate is currently 0.1.54, so downstream struct construction/patterns within the 0.1 compatible range can break), even though the field is serde(skip) and is not part of the versioned report wire contract. #362 only requires provider mapped output. Remove this breaking field and carry the provider-only mapping through a non-breaking path/API.

Attribute macro-expanded spans to the invocation site in a separate
lowering used only for the mapped artifact, so diagnostics and lowering
keep their definition spans. Return the mapped text beside the compile
report instead of adding a public CompileResult field.

Refs #362
@e54-bot

e54-bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 8e0a31d addressing findings 2 and 3.

  • 2: macro relocation now runs only in a second lowering for the mapped artifact; shared lowering and diagnostics are unchanged (test added).
  • 3: removed CompileResult::mapped; compile_source_report_mapped_with_language returns (CompileReport, Option<MappedText>), a new method, so nothing existing breaks.
  • 1: not changed. LPP 1.4 spec section 10.3 (language-provider-protocol branch feat/lpp-1.4-artifact-format-negotiation) refuses only when the provider would otherwise return a non-null artifact, and returns error diagnostics normally otherwise; the mock provider does the same. Added a test for the failing-source case. If you want the stricter behavior, the spec needs to change first.

@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

@Teakowa
Teakowa merged commit de6519e into main Sep 24, 2026
5 checks passed
@Teakowa
Teakowa deleted the feat/362-mapped-text branch September 24, 2026 19:20
This was referenced Sep 24, 2026
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.

Emit workshop-rs/mapped-text-v1 from opy-provider

2 participants