Align transformer spec, add tests, update docs

[leynos]

Summary

• Align transformer declaration grammar with the syntax spec by requiring a colon and an outputs list.
• Add tests to cover the new grammar and recovery behavior.
• Update docs to reflect the changes and update roadmap status.

Changes

Documentation updates

• docs/differential-datalog-parser-syntax-spec-updated.md

  • Transformer grammar now defined as:
    Transformer ::= 'extern' 'transformer' Ident '(' Params? ')' ':' TransformerOutputs
    TransformerOutputs ::= Ident (',' Ident)*
  • Clarified that transformer names and output names use a generic Ident token class and that the outputs list is required.
  • Noted line-oriented recovery: malformed transformer declarations recover by skipping to the next newline.

• docs/parser-conformance-register.md

  • Transformer declaration section updated to reflect:
    • Transformer names and outputs parsed via the generic Ident helper
    • Outputs require ':' and at least one output
    • Recovery behavior described (skip to next newline)
  • Marked the implemented status for this alignment.

• docs/parser-implementation-notes.md

  • Added note on line-oriented declaration recovery and that transformer output requirements are enforced directly in the transformer_decl grammar via .at_least(1).

• docs/roadmap.md

  • 2.6.5: Align transformer declaration grammar, including output signature requirements - now marked as implemented.

Tests

• tests/apply_items.rs
  • Import introduced: use ddlint::test_util::assert_parse_error;
  • Added test: transformer_declarations_require_a_colon_before_outputs
    • Verifies that missing colon after extern transformer yields a parse error and that no transformers are parsed.

Test plan

• Run cargo test locally
• Validate that transformer declarations without a colon fail with a parse error
• Ensure no regressions in unrelated parsing tests

◳ Generated by DevBoxer ◰

---

ℹ️ Tag @devboxerhub to ask questions and address PR feedback

📎 Task: https://www.devboxer.com/task/f3212383-3f9c-465d-8b96-358d66bfc8de

📝 Closes #249

Summary by Sourcery

Align transformer declaration grammar and parser behavior with the documented syntax and mark the roadmap item as implemented.

Enhancements:

• Document the transformer declaration grammar to require a colon and at least one output identifier, using generic identifiers for transformer names and outputs, and describe line-oriented recovery semantics in implementation notes.
• Update the parser conformance register to reflect the implemented transformer grammar, identifier handling, and recovery behavior.

Documentation:

• Revise the differential datalog parser syntax spec to define the new transformer grammar shape and clarify identifier and recovery behavior.
• Mark the transformer grammar alignment roadmap item as completed.

Tests:

• Add a behavioral test ensuring extern transformer declarations without a colon before outputs produce a parse error and no transformers are recorded.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4dada32b-19b3-4ff0-9154-29c940103666

📥 Commits

Reviewing files that changed from the base of the PR and between bf2d435 and 983081b.

📒 Files selected for processing (1)

• tests/apply_items.rs

---

Align transformer declaration grammar with documented syntax

Overview

This PR (closes #249) aligns the transformer declaration grammar, docs and tests with the intended colon-delimited, output-list-required form and records the parser's recovery behaviour.

Key changes

• Grammar: transformer declarations now require the form
  extern transformer Ident '(' Params? ')' ':' TransformerOutputs
  where TransformerOutputs is a comma-separated, at-least-one list of Ident; transformer and output names use the generic Ident token (no case-class enforcement).
• Tests: adds integration tests in tests/apply_items.rs (rstest-driven) to assert missing-colon declarations produce a chumsky SimpleReason::Unexpected expecting SyntaxKind::T_COLON and that malformed single transformers yield zero parsed transformers while later well-formed declarations are preserved.
• Documentation:
  • docs/differential-datalog-parser-syntax-spec-updated.md — defines the transformer grammar, clarifies Ident usage for names/outputs, documents line-oriented recovery (skip to newline on malformed declarations) and updates AST crosswalk with outputs field on TransformerDef.
  • docs/parser-conformance-register.md — updates the transformer entry to require ':' plus ≥1 outputs, documents generic Ident handling, recovery behaviour, and marks the item implemented.
  • docs/parser-implementation-notes.md — records the line-oriented recovery invariant (SpanCollector::skip_line()) and that outputs are enforced via the chumsky grammar (.at_least(1)) rather than a separate token-walking heuristic.
  • docs/roadmap.md — marks task 2.6.5 (align transformer declaration grammar) as completed.
• Commit/test metadata: PR from branch align-transformer-spec-docs-testing-rtfg1x into main; tests exercised locally (cargo test), validated missing-colon behaviour and no regressions in unrelated parsing tests.

Issues / follow-ups

• The PR documents that the implementation uses generic Ident tokens (resolving the LcName/UcName mismatch) rather than adding parser-level case checks.
• The duplicate diagnostic-paths issue (separating deterministic missing-output diagnostics from grammar-aware errors) is acknowledged but not refactored here and remains for future work.

Impact

• Specification, implementation notes and test-suite aligned to the same contract (':' plus at least one output, generic Ident usage, single-line recovery).
• No public API or exported-entity signature changes.

Walkthrough

Update transformer-declaration docs and tests: grammar requires : plus at least one output identifier; parser accepts generic non-keyword identifiers for transformer and output names; malformed declarations recover by skipping to the next newline; AST gains an outputs field; behavioural test for missing-colon form added and roadmap entry marked complete.

Changes

Cohort / File(s)	Summary
Specification updates docs/differential-datalog-parser-syntax-spec-updated.md	Change transformer declaration to extern transformer Ident '(' Params? ')' ':' TransformerOutputs; document generic non-keyword identifier usage for names, require : and at least one output, and specify line-oriented recovery on malformed declarations.
Conformance register docs/parser-conformance-register.md	Record parser behaviour for "12. Transformer declaration grammar": generic identifier token classification for names, enforce .at_least(1) on outputs in chumsky grammar, newline-bounded recovery; mark decision implemented.
Implementation notes docs/parser-implementation-notes.md	Add invariant: scanners call SpanCollector::skip_line() when declaration heads fail or when parse_and_record() does not consume a declaration; document that transformer_decl() enforces required outputs via chumsky grammar.
Roadmap docs/roadmap.md	Mark task 2.6.5 ("Align transformer declaration grammar, including output signature requirements") as completed.
Tests tests/apply_items.rs	Add behavioural test coverage for missing-colon transformer form: assert a single chumsky::error::SimpleReason::Unexpected expecting SyntaxKind::T_COLON, verify no transformer recorded for the malformed single-declaration case, and verify later well-formed declarations still parse.

Poem

📋 The spec now sings in tidy rows,
Colons placed and outputs shown,
Recovery skips to line’s repose,
Tests guard what the parser knows,
Docs ticked off — the roadmap grows.

🚥 Pre-merge checks | ✅ 8 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Testing	⚠️ Warning	Primary grammar enforcement is properly tested, but no explicit regression test validates uppercase-initial transformer names despite resolving specification gap #249.	Add dedicated test case validating uppercase-initial transformer names (e.g. extern transformer MyTransformer(input: T): Output). Extend recovery behaviour testing to cover consecutive malformed declarations. Implement property-based tests for diverse identifier naming patterns.

✅ Passed checks (8 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Align transformer spec, add tests, update docs' accurately summarizes the main changes: alignment of transformer grammar specification, addition of tests, and documentation updates across multiple files.
Description check	✅ Passed	The pull request description provides clear, structured details on all changes: transformer grammar alignment, test additions, and comprehensive documentation updates across four files with specific rationale for each change.
Linked Issues check	✅ Passed	The pull request addresses all four recommendations from issue #249: resolves the LcName/Ident mismatch by updating spec to match implementation, documents line-oriented recovery semantics, adds integration-level test for missing-colon form with proper assertions, and documents multiline declaration policy in implementation notes.
Out of Scope Changes check	✅ Passed	All changes remain within scope: documentation updates directly serve the grammar alignment objective, tests validate the new grammar requirements, and roadmap entry reflects completion of the tracked task.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
User-Facing Documentation	✅ Passed	PR updates developer documentation and tests for Differential Datalog parser specification, scoped to implementers and linter/IDE authors, not end-users. Phase 2 deliverable with end-user documentation scheduled for Phase 3.
Developer Documentation	✅ Passed	PR modifies only documentation and test files; no source code changes in src/ directory, no public APIs modified, no architectural boundaries or tooling requirements introduced.
Module-Level Documentation	✅ Passed	The pull request modifies only documentation files and appends test cases to an existing test module with appropriate module-level documentation already in place.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #249

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch align-transformer-spec-docs-testing-rtfg1x

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sourcery-ai]

Reviewer's Guide

Aligns the documented transformer declaration grammar with the implemented parser behavior (colon + required outputs, generic identifiers, line-based recovery) and adds a regression test ensuring declarations missing the colon fail to parse and produce no transformers.

Sequence diagram for transformer declaration parsing and line-oriented recovery

sequenceDiagram
    participant SourceFile
    participant TransformerScanner
    participant ChumskyTransformerGrammar
    participant SpanCollector

    SourceFile->>TransformerScanner: read_next_declaration()
    TransformerScanner->>ChumskyTransformerGrammar: transformer_decl(tokens)
    ChumskyTransformerGrammar->>ChumskyTransformerGrammar: match extern transformer Ident ( Params? )
    ChumskyTransformerGrammar->>ChumskyTransformerGrammar: expect ':' followed by outputs.at_least(1)
    alt missing_colon_or_outputs
        ChumskyTransformerGrammar-->>TransformerScanner: ParseError
        TransformerScanner->>SpanCollector: skip_line()
        SpanCollector-->>TransformerScanner: line_skipped
        TransformerScanner-->>SourceFile: record_parse_error_no_transformer
    else valid_transformer_declaration
        ChumskyTransformerGrammar-->>TransformerScanner: TransformerNode
        TransformerScanner-->>SourceFile: record_transformer
    end

Loading

File-Level Changes

Change	Details	Files
Align transformer grammar docs with the implemented parser, including required outputs, identifier handling, and recovery semantics.	Update transformer EBNF to require ':' followed by a TransformerOutputs list and define TransformerOutputs as one-or-more identifiers separated by commas. Document that transformer names and output names use the generic identifier token class (no case-based name class like UcName/LcName). Describe that malformed transformer declarations are recovered by skipping tokens to the next newline boundary. Update the parser conformance register to match current behavior, including generic identifier acceptance, .at_least(1) enforcement for outputs, and single-line recovery; mark decision status as implemented. Mark roadmap item 2.6.5 (transformer declaration grammar alignment) as implemented.	docs/differential-datalog-parser-syntax-spec-updated.md docs/parser-conformance-register.md docs/parser-implementation-notes.md docs/roadmap.md
Add a regression test that transformer declarations without a colon before outputs produce a parse error and no transformers are recorded.	Import the assert_parse_error helper from ddlint::test_util in the apply_items test module. Add a test case that parses an extern transformer missing the colon before outputs, asserts an 'Unexpected' parse error over the full input span, and checks that the parsed AST contains no transformers.	tests/apply_items.rs

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#249	Align the transformer declaration specification and documentation with the implementation, including using a generic identifier token (rather than LcName) for transformer names/outputs and explicitly documenting the single-line / line-oriented recovery policy.	✅
#249	Ensure the deterministic transformer diagnostic for missing output signatures is driven from the main grammar-aware parsing path, rather than a separate token-walking heuristic that can diverge from the accepted grammar.	✅
#249	Add an integration-level behavioural regression test that verifies transformer declarations missing the colon before outputs are rejected with an appropriate parse error and do not produce any parsed transformers.	✅

Possibly linked issues

• #: PR resolves all listed transformer follow-ups: Ident vs LcName, single grammar path, missing-colon test, and multiline docs.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/differential-datalog-parser-syntax-spec-updated.md`:
- Around line 234-237: The AST crosswalk entry for the Transformer node must be
updated to match the new grammar by adding a field for transformer outputs:
locate the AST crosswalk entry for Transformer (the Transformer node/class/AST
mapping referenced around the existing Transformer grammar) and add an outputs
(or transformerOutputs) field that exposes TransformerOutputs as a list of Ident
(i.e. the comma-separated output identifiers); update any references in the
crosswalk description to include this new outputs field so the spec text aligns
with the grammar.

In `@tests/apply_items.rs`:
- Around line 28-35: Extend the test
transformer_declarations_require_a_colon_before_outputs to verify
newline-bounded recovery: after the malformed line "extern transformer
missing_colon(input: SomeData)" add a newline and then a well-formed transformer
declaration (e.g., "extern transformer good_one(input: SomeData): OutputType")
so parsing should resume; call parse(src) on the combined source, keep the
existing assert_parse_error(parsed.errors(), ...) for the first error, then
assert that parsed.root().transformers() contains the recovered declaration
(check count > 0 and that one transformer has the identifier "good_one" or
equivalent) to prove recovery stops at the newline and records the second
declaration.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5af083b6-cb6b-4daf-b854-db4e98e3fe54

📥 Commits

Reviewing files that changed from the base of the PR and between ae5f0bf and bd213de.

📒 Files selected for processing (5)

• docs/differential-datalog-parser-syntax-spec-updated.md
• docs/parser-conformance-register.md
• docs/parser-implementation-notes.md
• docs/roadmap.md
• tests/apply_items.rs

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/apply_items.rs`:
- Around line 29-80: Two tests
(transformer_declarations_require_a_colon_before_outputs and
malformed_transformer_does_not_prevent_parsing_subsequent_declarations)
duplicate the same missing-colon assertions; refactor by adding a helper
function (e.g., assert_missing_colon_error(errors, expected_found)) that
encapsulates the SimpleReason/SyntaxKind::T_COLON checks and then replace the
two tests with a single #[rstest] parameterised test (e.g.,
transformer_missing_colon_behaviour) that takes (src, expected_found,
expected_names) and asserts the missing-colon error via
assert_missing_colon_error and then checks parsed.root().transformers() names;
also add use rstest::rstest import and adjust name mapping to compare
Option<String> values as in the proposed diff.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 68593635-eaba-439a-b7d2-a45acc8cf6c7

📥 Commits

Reviewing files that changed from the base of the PR and between bd213de and bf2d435.

📒 Files selected for processing (2)

• docs/differential-datalog-parser-syntax-spec-updated.md
• tests/apply_items.rs

[leynos]

@coderabbitai Have the following now been resolved?

Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity.

Please address the comments from this code review:

## Overall Comments
- The `transformer_declarations_require_a_colon_before_outputs` test currently asserts on the generic "Unexpected" message and full-span range, which may be brittle against future diagnostic wording or span refinement; consider asserting on a more specific error code/kind or a narrower span that captures just the missing-colon location.

## Individual Comments

### Comment 1
<location path="tests/apply_items.rs" line_range="29-34" />
<code_context>
 }
+
+#[test]
+fn transformer_declarations_require_a_colon_before_outputs() {
+    let src = "extern transformer missing_colon(input: SomeData)";
+    let parsed = parse(src);
+
+    assert_parse_error(parsed.errors(), "Unexpected", 0, src.len());
+    assert!(parsed.root().transformers().is_empty());
+}
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for the documented line-oriented recovery behavior when a malformed transformer is followed by a valid declaration.

Please also add a test that exercises the documented line-oriented recovery: a malformed transformer on the first line (e.g., missing colon) followed by a valid transformer on the next line. The test should verify that an error is reported only for the first line, the second transformer is parsed successfully, and later declarations remain unaffected, to ensure the implementation matches the documented recovery behavior.

Suggested implementation:

```rust
#[test]
fn transformer_declarations_require_a_colon_before_outputs() {
    let src = "extern transformer missing_colon(input: SomeData)";
    let parsed = parse(src);

    assert_parse_error(parsed.errors(), "Unexpected", 0, src.len());
    assert!(parsed.root().transformers().is_empty());
}

#[test]
fn malformed_transformer_does_not_prevent_parsing_subsequent_declarations() {
    let src = r#"extern transformer missing_colon(input: SomeData)
extern transformer ok(input: SomeData): OtherData
extern transformer another(input: OtherData): FinalData"#;

    let parsed = parse(src);
    let errors = parsed.errors();

    // We expect exactly one error, associated with the first malformed line.
    assert_eq!(errors.len(), 1);

    let first_line_len = src.find('\n').expect("source should contain a newline");
    assert_parse_error(errors, "Unexpected", 0, first_line_len);

    // Valid transformers after the malformed one should still be parsed.
    let transformers = parsed.root().transformers();
    assert_eq!(transformers.len(), 2);
}

```

If `parsed.root().transformers()` does not implement `.len()` and only exposes `is_empty()`, you may need to adapt the assertion to the available API (e.g., converting to a vec or slice first). Also, if the error API expects byte ranges in a specific way that differs from `[start, end)`, adjust the `first_line_len` usage accordingly. Finally, if your test utilities provide a helper for asserting “no more errors,” you can replace `assert_eq!(errors.len(), 1);` with that helper for consistency.
</issue_to_address>

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat tests/apply_items.rs

Length of output: 2297

---

@leynos Yes, both comments are resolved in the current branch.

Overall comment (brittleness): The original assert_parse_error call with a generic "Unexpected" string and full-span byte range has been replaced by the typed assert_missing_colon_error helper, which:

• pattern-matches on SimpleReason::Unexpected directly,
• asserts SyntaxKind::T_COLON is in the expected-token set, and
• compares error.found() against a typed Option<SyntaxKind>.

This is substantially more precise and robust against diagnostic-wording changes.

Individual comment 1 (recovery test): The transformer_missing_colon_behaviour #[rstest] test now has two parametrised cases:

1. A lone malformed declaration → expected_found = None, zero transformers.
2. A malformed first declaration followed by two valid ones → expected_found = Some(SyntaxKind::K_EXTERN), names ["ok", "another"] — exercising the documented line-oriented recovery contract.