Preserve per-entry changelog errors on parse failure and switch envoy.code.check to shared entry constants

[Copilot]

This follow-up tightens the per-entry changelog validation added in the earlier PR. The main issue was error masking: once section validation started raising during await self.data, AChangelogStatus.errors could return a single coarse parse error and drop the per-file filename/content errors that this checker is meant to surface.

• Error aggregation

  • Run check_entry_files() before the parse-dependent checks in AChangelogStatus.errors.
  • Preserve existing happy-path ordering.
  • On ChangelogParseError, return per-entry file errors first, then the coarse parse error.

  @async_property(cache=True)
  async def errors(self) -> tuple[str, ...]:
      entry_errors = await self.check_entry_files()
      try:
          return (
              *self.check_version(),
              *await self.check_date(),
              *await self.check_sections(),
              *entry_errors)
      except utils.exceptions.ChangelogParseError as e:
          return (*entry_errors, f"{self.version}: {e}")

• Shared changelog entry constants

  • Replace the local ENTRY_SEPARATOR / CHANGELOG_ENTRY_GLOB definitions in envoy.code.check.abstract.changelog.
  • Import both constants from envoy.base.utils.abstract.project.changelog, matching the landed dependency shape.

• Test rewrites

  • Rewrite the seven check_entry_filename unit tests to use fully mocked path objects (MagicMock) instead of pathlib.Path literals.
  • Only set the attributes the production code reads: parent.name, stem, and suffix.

• Ordering/call expectation updates

  • Update test_changelogstatus_errors to reflect the new failure ordering:
    • success: version → date → sections → entry files
    • parse failure: entry files → coarse parse error
  • Assert that check_entry_files() is invoked unconditionally, including when parsing later fails.

• Branch-local dependency alignment

  • Point py/envoy.code.check source/test targets at the local envoy.base.utils source target so the shared constants resolve from the in-repo implementation expected by this stacked change.

Original prompt

Context

This refines PR #4502 (branch copilot/featureper-entry-filename-validation), which adds per-entry filename and content validation to envoy.code.check.

The dependencies it was waiting on have now landed:

• py/envoy.base.utils: Add per-entry RST directory reader #4518 — per-entry RST directory reader in envoy.base.utils, exposing module-level constants ENTRY_SEPARATOR and CHANGELOG_ENTRY_GLOB in envoy.base.utils.abstract.project.changelog.
• py/envoy.base.utils: Switch to runtime validation for changelog sections #4519 — runtime validate_sections on AChangelogs, called from AChangelog.data. Any unknown section key in a parsed changelog now raises utils.exceptions.ChangelogParseError from await self.data, which check_date and check_sections both go through.

Three concrete changes needed.

1. Fix errors() ordering so per-file errors survive a parse failure

File: py/envoy.code.check/envoy/code/check/abstract/changelog.py

Current state on the branch:

@async_property(cache=True)
async def errors(self) -> tuple[str, ...]:
    try:
        return (
            *self.check_version(),
            *await self.check_date(),
            *await self.check_sections(),
            *await self.check_entry_files())
    except utils.exceptions.ChangelogParseError as e:
        return (f"{self.version}: {e}", )

After #4519, await self.data (called via check_date and check_sections) runs validate_sections, which raises ChangelogParseError on any unknown section key. With current ordering, this is caught and errors returns a single coarse message — the per-file errors from check_entry_files are never collected, defeating the purpose of this PR.

check_entry_files is parse-independent: it only globs the filesystem and uses self.checker.sections (loaded directly from sections.yaml, not via self.data). Hoist it before the try block:

@async_property(cache=True)
async def errors(self) -> tuple[str, ...]:
    entry_errors = await self.check_entry_files()
    try:
        return (
            *self.check_version(),
            *await self.check_date(),
            *await self.check_sections(),
            *entry_errors)
    except utils.exceptions.ChangelogParseError as e:
        return (*entry_errors, f"{self.version}: {e}")

Guarantees:

• Happy path: errors in existing order (version → date → sections → entry files).
• Parse failure: per-file errors first, then the coarse parse error.

2. Replace TODO local constants with imports from envoy.base.utils

Currently:

# TODO: Replace with imports from envoy.base.utils once PR 2 lands
ENTRY_SEPARATOR = "__"
CHANGELOG_ENTRY_GLOB = "*/*.rst"

PR #4518 landed; these constants exist in envoy.base.utils.abstract.project.changelog. Replace local definitions with an import — match the existing import style in this file (which already imports utils.typing / utils.exceptions). Direct import from envoy.base.utils.abstract.project.changelog is acceptable; do not aggressively re-architect re-exports.

Remove the TODO comment. Update any call sites still using the bare names (they should resolve via the import).

3. Rewrite check_entry_filename tests with fully-mocked paths

File: py/envoy.code.check/tests/test_abstract_changelog.py

The seven check_entry_filename tests added by #4502 use real pathlib.Path literals:

def test_changeschecker_check_entry_filename_valid():
    changelog = DummyChangelogChangesChecker({"bug_fixes": MagicMock()})
    path = pathlib.Path("changelogs/current/bug_fixes/myarea__myslug.rst")
    assert changelog.check_entry_filename(path) is None

The convention in adjacent test files (canonical example: test_abstract_changelog_get_data_from_entries_* in py/envoy.base.utils/tests/test_abstract_project_changelogs.py) is fully-mocked "ALL and ONLY" style.

Rewrite all seven tests:

• test_changeschecker_check_entry_filename_valid
• test_changeschecker_check_entry_filename_invalid_section
• test_changeschecker_check_entry_filename_wrong_extension
• test_changeschecker_check_entry_filename_no_separator
• test_changeschecker_check_entry_filename_multiple_separators
• test_changeschecker_check_entry_filename_empty_area
• test_changeschecker_check_entry_filename_empty_slug

In each:

• path = MagicMock() — do not construct pathlib.Path.
• Set the attributes the production code reads: path.parent.name, path.stem (real strings so .split / .count work), path.suffix.
• Return-value assertions unchanged (is None for valid; substring checks for the others).
• Keep substring assertions that check path.parent.name text ("weird_section" etc.) — they still work because the production code interpolates path.parent.name which we set explicitly.
• For assertions on the path itself appearing in error messages: either drop them (assert only on descriptive text like "Area" / "empty") or set path.__str__ so the resulting string is known. Prefer dropping unless the message would be meaningless without the path.

Pa...

This pull request was created from Copilot chat.