Skip to content

fix(template): guard optional property in nested test#113

Open
OmarElzero wants to merge 1 commit intoaccordproject:mainfrom
OmarElzero:omar/112/fix-optional-test
Open

fix(template): guard optional property in nested test#113
OmarElzero wants to merge 1 commit intoaccordproject:mainfrom
OmarElzero:omar/112/fix-optional-test

Conversation

@OmarElzero
Copy link

This pull request resolves a test failure in test/TemplateMarkInterpreter.test.ts that occurred after the stricter optional property checks were introduced in PR #53.

The should generate optional-nested test was failing with the error Optional properties used without guards: age. This was because the template was using an optional property (age) without a guard.

This fix wraps the age variable in the test/templates/good/optional-nested/template.md file with an {{#optional}} block, ensuring it conforms to the new validation rules. The corresponding test snapshot has also been updated.

Fixes #112

Copilot AI review requested due to automatic review settings March 21, 2026 06:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the optional-nested “good” template to satisfy the stricter optional-property type checking introduced in PR #53 by guarding access to the optional age field, and refreshes the interpreter snapshot accordingly.

Changes:

  • Wrap age usage in test/templates/good/optional-nested/template.md with an {{#optional}} guard.
  • Update the Jest snapshot to reflect the generated Optional node output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
test/templates/good/optional-nested/template.md Adds an {{#optional age}} guard around the age interpolation in the template.
test/__snapshots__/TemplateMarkInterpreter.test.ts.snap Updates expected generated AgreementMark/CiceroMark JSON (including Optional node structure and list tightness).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


- You are *{{age}}* years old
- You are *{{#optional age}}{{this}}{{/optional}}* years old

Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

The blank line between list items (after the age bullet) changes the CommonMark list from tight to loose (snapshot tight flips to false). Since this PR’s goal is only to add an optional guard, consider removing the blank line to preserve the original list structure/output and avoid an unrelated snapshot change.

Suggested change

Copilot uses AI. Check for mistakes.
{{/clause}}

- You are *{{age}}* years old
- You are *{{#optional age}}{{this}}{{/optional}}* years old
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

Wrapping only {{age}} means that when age is absent the template will still emit the bullet - You are ** years old (empty emphasis/content). To make the optional handling render cleanly, wrap the whole list item in the {{#optional age}}...{{/optional}} block or provide an {{else}} branch with fallback text.

Suggested change
- You are *{{#optional age}}{{this}}{{/optional}}* years old
{{#optional age}}- You are *{{this}}* years old{{/optional}}

Copilot uses AI. Check for mistakes.
The optional-nested test was failing after stricter optional property checks were added. This change wraps the 'age' variable in the template with an optional guard to align with the model.

Fixes accordproject#112

Signed-off-by: OmarElzero <omarmohmed7659944@gmail.com>
@OmarElzero OmarElzero force-pushed the omar/112/fix-optional-test branch from bd7453d to 2005b0d Compare March 21, 2026 15:50
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.

Fix: "optional-nested" test fails after stricter optional checks in PR #53

2 participants