Add deterministic fixer for common YAML checker findings

[nonprofittechy]

Summary

Adds a single, deterministic, formatting-preserving fixer for four mechanical DAYamlChecker findings:

• EG414: adds a question block id derived from normalized question text; repeated generated IDs receive 2, 3, etc. while avoiding all existing IDs.
• EA510: expands yesno/noyes shortcuts to an explicit fields entry with datatype: yesnoradio. yesnomaybe/noyesmaybe are expanded with datatype: yesnomaybe to preserve their third answer choice.
• EA502: labels only the first offending input field with the question text, including blank mapping keys such as - "": variable. It does not guess labels for additional unlabeled fields on the same screen.
• EG104: suffixes later duplicate block IDs, including the corpus's common id: | block-scalar form, until IDs are unique within each YAML file.

The implementation lives in src/dayamlchecker/fixer.py, with each fix in its own function. The standalone scripts/fix_yaml_checker.py remains as a compatibility wrapper and dry-run entry point. The main dayamlchecker CLI now accepts --fix, writes only validated plans, and then runs the normal checker against the updated files. Rewritten mapping lines also have trailing whitespace removed. Without --fix, behavior is unchanged.

Validation

• Five focused fixer regression tests pass, covering all four rule families, duplicate question text, blank mapping-key labels, block-scalar IDs, semantic preservation for yesnomaybe, residual EA502 behavior, dry-run non-mutation, and idempotence.
• The full pytest suite passes: 322 tests and 54 subtests.
• Mypy passes on the source, test, and script tree; Black passes on all changed Python files.
• Dry-run against the checked-out Massachusetts snapshot: 54 repos, 180 YAML files, 89 files with safe plans, 383 planned edits (EG414 156, EA510 112, EA502 77, EG104 38). Four pre-existing parse-invalid YAML files were skipped; no candidate was rejected.
• Every planned candidate is reparsed and rerun through the checker before it can be written. A second dry run after applying the plans produced no further edits; screens with multiple unlabeled fields intentionally leave later fields untouched.
• The CLI integration test confirms --fix writes the changes before the ordinary checker runs and reports the updated files clean.
• The exact CI commands pass from a clean branch checkout: mypy, black --check ., and pytest -q.

[Copilot]

🟡 Changes recommended

Unresolved critical fixer defects can produce invalid YAML, misapply labels, or leave findings unfixed.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds deterministic, formatting-preserving YAML fixes for four checker findings, with CLI integration, validation, tests, and documentation.

Changes:

• Implements fixer planning, validation, dry-run, and write support.
• Adds --fix CLI integration and preserves the standalone wrapper.
• Adds regression tests and README documentation.

File summaries

File	Review summary
tests/test_yaml_structure_cli.py	No final comments.
tests/test_fix_yaml_checker.py	No final comments.
src/dayamlchecker/yaml_structure.py	No final comments.
src/dayamlchecker/fixer.py	Critical (2 votes): sequence-field label insertion creates an extra list item; add regression coverage. Critical (1 vote): multiple shortcuts can create duplicate fields keys. Moderate (3 votes): select the first actual EA502 offender, not only the first labelable field. Nit (1 vote): preserve CRLF line endings. Critical (1 vote): reject candidates containing parse findings such as duplicate keys. Critical (1 vote): correctly handle folded block scalars. Moderate (1 vote): match EG104 findings to the full document range. Critical (1 vote): do not rewrite boolean no label markers as labels. Moderate (1 vote): insert labels after complete block or collection values. Moderate (1 vote): resolve block ID keys case-insensitively. Nit (1 vote): count only edits from validated plans.
scripts/fix_yaml_checker.py	No final comments.
README.md	No final comments.

Review details

Suppressed comments (5)

src/dayamlchecker/fixer.py:618

• Path.read_text uses universal-newline translation, so CRLF input is already converted to LF before _newline_for runs; apply_plan reads it the same way. Any changed file therefore loses its original line endings despite this fixer claiming to preserve formatting. Read raw text with newline="" (or decode read_bytes()) in both reads.

        text = path.read_text(encoding="utf-8")

src/dayamlchecker/fixer.py:358

• EG104 is reported at the document start (block_start in yaml_structure.py), but document.lc.line points at the first mapping key. With comments or blank lines immediately after ---, those locations differ by more than one line, so this duplicate is not selected for editing and remains in the candidate. Match the finding to the actual document range rather than allowing only a ±1-line heuristic.

    # Depending on the document's first key and surrounding ``---`` marker,
    # the checker may report the marker, the first key, or the adjacent line.
    return bool({start - 1, start, start + 1}.intersection(target_lines))

src/dayamlchecker/fixer.py:592

• The label is inserted immediately after the first key line, rather than after that key's value. When the first key has a block or collection value such as choices: or datatype: |, this places label inside that value; candidate validation rejects the whole file, so unrelated safe fixes in the same file are also lost. Insert after the complete value or skip only this EA502 edit.

        _insert_before_line(
            lines,
            line_number=key_location[0] + 1,
            replacement_lines=[f"{indent}label: {_quote_yaml_string(question)}"],

src/dayamlchecker/fixer.py:430

• The checker accepts block-ID keys case-insensitively (_get_case_insensitive), so a valid ID: key can produce EG104. This exact lookup leaves current_id empty and skips the duplicate; the normal checker then still reports EG104 after --fix. Resolve the ID key case-insensitively for both the value and source location.

    current_id = (
        str(document.get("id")).strip() if isinstance(document.get("id"), str) else ""
    )

src/dayamlchecker/fixer.py:750

• plan.counts is populated before candidate validation. If a candidate is rejected, plan.changed is false and nothing is written, but this loop still adds its counts, causing the CLI to report rejected edits under Fixes by rule. Aggregate counts only for validated plans.

    counts: Counter[str] = Counter()
    for plan in plans:
        counts.update(plan.counts)

• Files reviewed: 6/6 changed files
• Comments generated: 6
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.