da_build: check DOCX templates for accessibility, as warnings by default - #94
Merged
Merged
Conversation
The YAML checker step only ever fed *.yml under questions/ to dayamlchecker, so its DOCX accessibility checks never saw a file. Expand the same find to pick up *.docx under data/templates. Deliberately still matching specific files rather than passing a directory: not every YAML file in these repos is a docassemble interview (workflows, ALKiln fixtures), and only documents under data/templates are the ones users actually receive, so widening to whole directories would add a new class of false positives. Findings are reported at warning severity by default, which annotates the job without failing it -- most templates have findings today and authors need room to work through them. Repos opt into failing with docx-validation-mode: error, mirroring verapdf-validation-mode. skip-docx-check turns the checks off entirely. Requires dayamlchecker 1.5.0 for --docx-accessibility-severity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4YaH5h8wS9Kb5uveGQhyd
One input instead of two, with 'off' as a third mode alongside 'warning' and 'error'. Also anchors the docx glob at ./docassemble/*/data/templates so a local .venv full of other packages' templates is never picked up, and passes --format github so findings annotate the PR instead of only reaching the log.
The `uv build` step copies the whole package into build/lib before the checker runs, so `-path "*/questions/*"` matched every question file twice. A real CI run showed the consequence: all 20 error and warning annotations pointed at build/lib copies, so none of them attached to a file in the PR diff, and the source-file duplicates lost the race for the slots. Prune build/, dist/ and .venv, and anchor both patterns at ./docassemble/*.
nonprofittechy
requested review from
BryceStevenWilley
and
a lite review from Copilot
August 31, 2026 19:20
There was a problem hiding this comment.
🟡 Changes recommended
The new find ... | xargs ... python -m dayamlchecker pipeline can invoke dayamlchecker with zero input files, which will fail the action in repos/branches without matching YAML/DOCX files.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the da_build composite GitHub Action to run DAYamlChecker against both interview YAML files and DOCX templates, enabling DOCX accessibility findings to be surfaced as GitHub annotations (warnings by default) while also avoiding duplicate findings from build artifacts.
Changes:
- Adds a new
docx-validation-modeinput (defaultwarning) and wires it to DAYamlChecker DOCX accessibility flags. - Expands the file discovery to include
*.docxunderdocassemble/*/data/templates/*, and sets output formatting to GitHub annotations. - Prunes
build/,dist/, and.venvfrom the checker’sfindto prevent duplicate reporting fromuv buildcopies.
File summaries
| File | Description |
|---|---|
| da_build/action.yml | Adds configurable DOCX accessibility checking and improves file selection to avoid duplicate annotations from build artifacts. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
BryceStevenWilley
approved these changes
Sep 1, 2026
BryceStevenWilley
left a comment
BryceStevenWilley
left a comment
Contributor
There was a problem hiding this comment.
LGTM, comments on some cruft that doesn't really do anything but good either way.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on SuffolkLITLab/DAYamlChecker#82 (needs dayamlchecker 1.5.0 published).
The YAML checker step only ever fed
*.ymlunderquestions/to dayamlchecker, so its DOCX accessibility checks never saw a file. This expands the samefindto pick up*.docxunderdata/templates.Still matching specific files rather than passing a directory: not every YAML file is a docassemble interview (workflows, ALKiln fixtures), and only documents under the package's own
data/templatesare the ones users receive, so widening to whole directories would add a new class of false positives.New input
Defaults to
warning, which annotates the job without failing it — most templates have findings today and authors need room to work through them.Also fixes a pre-existing duplication bug
uv buildcopies the whole package intobuild/libbefore the checker runs, so-path "*/questions/*"matched every question file twice. A real CI run showed the consequence: all 20 error and warning annotations pointed atbuild/libcopies, so none attached to a file in the PR diff and the real source files lost the race for GitHub's 10-annotations-per-level cap. Pruningbuild/,dist/and.venvtook that run from 188 findings to 108.Verified in real CI
LemmaLegalConsulting/docassemble-MOHUDEvictionProject#669 — 14 templates, 35 findings (19 warnings, 16 notices, 0 errors), annotations attached to the right paths, and the build's failure came entirely from pre-existing YAML issues.