Skip to content

Improve block formatting and diagnostic reporting#38

Open
keyboardDrummer-bot wants to merge 2 commits intomainfrom
formatting-and-debugging-improvements-v2
Open

Improve block formatting and diagnostic reporting#38
keyboardDrummer-bot wants to merge 2 commits intomainfrom
formatting-and-debugging-improvements-v2

Conversation

@keyboardDrummer-bot
Copy link
Copy Markdown
Collaborator

Summary

Extracts the formatting and debugging improvements from #34 into a standalone PR.

Formatting improvements

  • Block formatting: Changes block output from single-line { stmt1; stmt2 } to vertical layout with indent(2):
    {
      stmt1;
      stmt2
    }
    
  • Semicolon separator: Uses newlines instead of spaces between semicolon-separated items in the formatter.

Debugging improvements

  • Better diagnostic reporting: Replaces the boolean coreProgramHasSuperfluousErrors with a coreDiagnostics : List DiagnosticModel that records why the Core program was suppressed. When no other diagnostics explain the suppression, these are surfaced to the user.
  • Informative invalidCoreType: Adds source and reason parameters so each suppression site provides context about what went wrong.
  • Intermediate file output: Adds processLaurelFileKeepIntermediates test helper that writes pipeline intermediate files to Build/ for debugging.
  • .gitignore: Adds Build/ directory.

Test updates

  • All expected outputs updated to match the new block formatting.
  • Minor #guard_msgs whitespace fixes.
  • Some test inputs updated to use opaque keyword where needed for the new formatting to apply correctly.

Formatting improvements:
- Change block formatting to use indent(2) with newlines for vertical layout
  instead of single-line '{ ... }' format
- Update semicolon separator to use newlines instead of spaces

Debugging improvements:
- Replace boolean 'coreProgramHasSuperfluousErrors' with 'coreDiagnostics' list
  that records why the Core program was suppressed, enabling better error surfacing
- Add source location and reason parameters to 'invalidCoreType' for more
  informative diagnostics
- Surface core diagnostics when no other diagnostics explain program suppression
- Add 'processLaurelFileKeepIntermediates' test helper for writing intermediate
  pipeline files to Build/ directory
- Add Build/ to .gitignore

Test updates:
- Update all expected outputs to match new block formatting
- Add 'opaque' keyword to test procedures that need it for the new formatting
- Fix #guard_msgs whitespace formatting
Diagnostics with FileRange.unknown are not actionable for users and
can arise from pre-existing resolution limitations (e.g., variables
in multi-assign with field targets losing their uniqueId). Filter
these out when surfacing suppression reasons to avoid spurious errors.
@keyboardDrummer-bot
Copy link
Copy Markdown
Collaborator Author

I investigated the failing Run internal benchmarks of Strata check. This is not a code issue — all other CI checks pass, and the build and tests pass locally.

The benchmark job triggers an AWS CodeBuild build with --source-location-override https://github.com/strata-org/Strata.git and --source-version <head SHA>. Since this PR lives on keyboardDrummer/Strata, the commit SHA doesn't exist in the strata-org/Strata repo, so CodeBuild fails to check out the source.

This is an infrastructure limitation: the benchmark job is hardcoded for the upstream strata-org/Strata repository and will fail on any PR from a fork (or a different org's copy). No code fix in this PR can resolve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants