Skip to content

bug: Keep report serialization from mutating check metadata #217

Description

@gkapfham

Summary

Prevent report generation from mutating the check configuration stored inside
CheckResult objects. The current implementation of
gatorgrade/output/output.py:create_report_json() assigns generated fields
such as status, weight, outputlimit, diagnostic, hint, and check_id
directly into each result's json_info dictionary.

Problem

json_info is also held by the configured ShellCheck or
GatorGraderCheck. Because the dictionary is modified in place:

  • generating a report changes the source check configuration;
  • calling create_report_json() twice can retain stale fields from an earlier
    result or run;
  • a reused dictionary can retain a previous diagnostic even when the next
    result passes;
  • a later Markdown or JSON report can depend on which report was generated
    first; and
  • callers cannot safely compare the original configuration with the executed
    result.

The existing code only adds a path when it is truthy and only adds a diagnostic
when a check fails, so stale optional fields are especially easy to preserve.

Suggested implementation

Build a fresh serializable dictionary for every check, for example by making a
deep copy of dictionary metadata and then explicitly setting or removing all
runtime fields. Do not rely on the input dictionary already having a clean
state. Consider a dedicated report serializer that separates configuration
fields from runtime fields and consistently handles non-dictionary json_info.

Define whether runtime fields such as diagnostic, hint, and path should be
omitted or set to null when they do not apply. Keep the JSON schema stable and
document the decision. Ensure the history writer receives the clean serialized
report rather than mutable execution state.

Acceptance criteria

  • create_report_json() does not change any input CheckResult or check
    configuration dictionary.
  • Two calls with the same results produce equivalent JSON apart from any
    intentionally generated report timestamp.
  • A failed result followed by a passing result cannot leak diagnostic, hint, or
    path data into the passing result.
  • The behavior is correct for shell checks, GatorGrader checks, missing paths,
    non-dictionary metadata, and explicit hints.
  • Regression tests assert both the generated JSON and that the original
    dictionaries are unchanged.

Reproduction and regression test

The problem can be reproduced with a focused test in
tests/output/test_output.py:

metadata = {"description": "same check"}
failed = CheckResult(
    False,
    "same check",
    metadata,
    diagnostic="old diagnostic",
    check_id="check-1",
)
passing = CheckResult(
    True,
    "same check",
    metadata,
    diagnostic="new diagnostic",
    check_id="check-1",
)
first_report = create_report_json(0, [failed], 0)
second_report = create_report_json(1, [passing], 100)
assert "diagnostic" not in second_report["checks"][0]
assert "status" not in metadata

The final assertion currently fails because report construction writes runtime
fields into metadata. The test should also assert that a failed report's
metadata is not changed and that repeated serialization is independent.

Contributor verification

  • Add focused unit tests before changing the serializer.
  • Run the relevant output tests with uv run pytest tests/output/test_output.py.
  • Run uv run task all from the GatorGrade repository before considering the
    fix complete. All lint, type, test, coverage, and direct-coverage tasks must
    pass; update tests and documentation if the schema changes.
  • Keep the change separate from reserved-check implementation work.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingoutputRequirement for Output team

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions