Skip to content

feat(arxjit): add structured diagnostics and the public error hierarchy#95

Open
Jaskirat-s7 wants to merge 1 commit into
arxlang:mainfrom
Jaskirat-s7:feat/arxjit-diagnostics
Open

feat(arxjit): add structured diagnostics and the public error hierarchy#95
Jaskirat-s7 wants to merge 1 commit into
arxlang:mainfrom
Jaskirat-s7:feat/arxjit-diagnostics

Conversation

@Jaskirat-s7

Copy link
Copy Markdown
Contributor

What

First PR of arxjit Sprint 2 (source extraction + Python AST validation), per the 4-PR split posted in the Discord thread:

  • arxjit.diagnostics — frozen Diagnostic record (severity, message, filename, line, column, code) + DiagnosticSeverity enum (ERROR/WARNING/INFO/HINT), with a human-readable __str__ (file:line:col: error: [code] message).
  • arxjit.errorsArxJitError base carrying list[Diagnostic], with SourceUnavailableError (source cannot be retrieved and parsed as a standalone function definition: REPL, exec, C functions, lambdas) and UnsupportedSyntaxError (constructs outside the v1 subset), mapping 1:1 to the two Sprint 2 layers.
  • Both exported from the package root; 16 new tests (arxjit coverage stays 100%).

Why diagnostics before extraction/validation

Both upcoming layers report user-facing failures — extraction fails on source-less functions, and validation's whole job is rejecting unsupported syntax with clear messages (wiki Issue 4). Landing the shared vocabulary first (~90 lines) means both PRs raise the same typed errors from day one instead of ad-hoc strings retrofitted later.

Design notes

  • Same 6-field diagnostic contract as pyarx (reviewed in feat(pyarx): add error and diagnostic layer #86), so both packages speak one convention — but no cross-import: arxjit stays standalone, and none of pyarx's irx-translation machinery is carried over. arxjit diagnostics point at the user's real Python file/line via ast nodes.
  • code (e.g. AJ001) is optional and unassigned for now; the validation PR can introduce stable codes if wanted.
  • Per the Discord decision: default behavior on rejected functions will be warn + fall back to plain Python, with a boolean strict-style parameter on @jit to force-raise — that lands in the validation wiring PR, where these exceptions get raised.

Checks

  • pytest: 33 passed, arxjit coverage 100% (gate 86)
  • ruff format + check, mypy strict (mypy src), bandit, vulture, mccabe: clean
  • douki sync run and idempotent (second run: 0 updated)

Add arxjit.diagnostics (frozen Diagnostic record + DiagnosticSeverity,
same 6-field contract as pyarx) and arxjit.errors (ArxJitError base
carrying a list of diagnostics, with SourceUnavailableError and
UnsupportedSyntaxError for the Sprint 2 source-extraction and
validation layers). Export both from the package root; 15 new tests.
@Jaskirat-s7 Jaskirat-s7 force-pushed the feat/arxjit-diagnostics branch from 08a65a0 to d81cb52 Compare July 11, 2026 18:59

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

Review verdict: request changes

The implementation is clean and CI is green, but I found one public-API issue that should be resolved before merging.

Blocking: define how AST columns are normalized

Diagnostic.column is documented as a one-based source column. However, the upcoming validation layer will obtain locations from Python AST nodes, whose col_offset is a zero-based UTF-8 byte offset, not a one-based character column. ([Python documentation][1])

Simply storing node.col_offset violates the documented contract. Adding one fixes ASCII positions but can still produce incorrect visual columns when non-ASCII characters precede the node.

Because this PR intentionally establishes the public diagnostic contract before extraction and validation land, I recommend choosing one precise representation now:

  • Prefer a one-based Unicode character column for human-readable diagnostics.
  • Convert AST byte offsets at the boundary rather than exposing them directly.
  • Add future integration tests covering an item at column zero and an item preceded by a non-ASCII character.
  • Alternatively, document it explicitly as a zero-based UTF-8 byte offset, though that is less suitable for the current human-readable __str__ output.

Non-blocking: clarify SourceUnavailableError

The class title says it represents source that “cannot be obtained,” but its summary also covers source that was obtained but cannot be parsed as a standalone function definition, including lambdas. Meanwhile, UnsupportedSyntaxError represents rejected Python constructs. This makes the boundary between these public exception types unclear.

Consider either:

  • renaming it to SourceExtractionError, or
  • reserving it for genuinely unavailable source and treating unsupported callable shapes as UnsupportedSyntaxError.

What looks good

The frozen diagnostic value object, defensive copying of diagnostic sequences, explicit package exports, and small exception hierarchy are straightforward and consistent with the existing arxjit pattern.

All three associated Actions workflows completed successfully.

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.

2 participants