Add dashboard release health summary artifact#30
Merged
ProfRandom92 merged 1 commit intoMay 11, 2026
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a dashboard release health summary system, including a new generation script and corresponding documentation updates. Feedback focuses on improving path consistency across artifact definitions, enhancing type safety by using TypedDict instead of generic dictionaries, and refactoring the status determination logic to be more robust and simplified.
Comment on lines
+70
to
+95
| OPTIONAL_CROSS_REPO_ARTIFACTS = [ | ||
| Artifact( | ||
| "benchmark_summary", | ||
| "benchmark-summary.json", | ||
| "Optional sanitized benchmark promotion summary from the experiment repository.", | ||
| False, | ||
| ), | ||
| Artifact( | ||
| "regression_summary", | ||
| "regression-summary.json", | ||
| "Optional sanitized regression promotion summary from the experiment repository.", | ||
| False, | ||
| ), | ||
| Artifact( | ||
| "sanitization_summary", | ||
| "sanitization-summary.json", | ||
| "Optional sanitized data-handling summary from the experiment repository.", | ||
| False, | ||
| ), | ||
| Artifact( | ||
| "report_contract_validation_report", | ||
| "report-contract-validation-report.md", | ||
| "Optional report-contract validation evidence from the experiment repository.", | ||
| False, | ||
| ), | ||
| ] |
Contributor
There was a problem hiding this comment.
The paths for optional artifacts are defined as filenames only, while local artifacts use full repository-relative paths. This leads to inconsistent path reporting in the generated JSON and Markdown artifacts. It is recommended to define all artifact paths relative to the repository root for consistency.
OPTIONAL_CROSS_REPO_ARTIFACTS = [
Artifact(
"benchmark_summary",
"docs/reports/benchmark-summary.json",
"Optional sanitized benchmark promotion summary from the experiment repository.",
False,
),
Artifact(
"regression_summary",
"docs/reports/regression-summary.json",
"Optional sanitized regression promotion summary from the experiment repository.",
False,
),
Artifact(
"sanitization_summary",
"docs/reports/sanitization-summary.json",
"Optional sanitized data-handling summary from the experiment repository.",
False,
),
Artifact(
"report_contract_validation_report",
"docs/reports/report-contract-validation-report.md",
"Optional report-contract validation evidence from the experiment repository.",
False,
),
]| ] | ||
|
|
||
|
|
||
| ArtifactRecord = dict[str, object] |
Contributor
There was a problem hiding this comment.
Using dict[str, object] for structured data results in several type: ignore comments because the type checker cannot verify the dictionary's structure. Since this script targets Python 3.11+, using TypedDict would provide better type safety and allow for cleaner code without manual type suppression.
Comment on lines
+125
to
+139
| def artifact_record(artifact: Artifact, base_dir: Path | None = None) -> ArtifactRecord: | ||
| """Build a deterministic artifact status record without reading contents.""" | ||
| absolute_path = (base_dir / artifact.path) if base_dir else (ROOT / artifact.path) | ||
| present = absolute_path.exists() | ||
| record: ArtifactRecord = { | ||
| "key": artifact.key, | ||
| "path": artifact.path, | ||
| "required": artifact.required, | ||
| "present": present, | ||
| "status": "present" if present else "missing", | ||
| "description": artifact.description, | ||
| } | ||
| if present and absolute_path.is_file(): | ||
| record["size_bytes"] = absolute_path.stat().st_size | ||
| return record |
Contributor
There was a problem hiding this comment.
By ensuring all artifact paths are repository-relative, the base_dir parameter can be removed, simplifying the logic for determining the absolute path.
Suggested change
| def artifact_record(artifact: Artifact, base_dir: Path | None = None) -> ArtifactRecord: | |
| """Build a deterministic artifact status record without reading contents.""" | |
| absolute_path = (base_dir / artifact.path) if base_dir else (ROOT / artifact.path) | |
| present = absolute_path.exists() | |
| record: ArtifactRecord = { | |
| "key": artifact.key, | |
| "path": artifact.path, | |
| "required": artifact.required, | |
| "present": present, | |
| "status": "present" if present else "missing", | |
| "description": artifact.description, | |
| } | |
| if present and absolute_path.is_file(): | |
| record["size_bytes"] = absolute_path.stat().st_size | |
| return record | |
| def artifact_record(artifact: Artifact) -> ArtifactRecord: | |
| """Build a deterministic artifact status record without reading contents.""" | |
| absolute_path = ROOT / artifact.path | |
| present = absolute_path.exists() | |
| record: ArtifactRecord = { | |
| "key": artifact.key, | |
| "path": artifact.path, | |
| "required": artifact.required, | |
| "present": present, | |
| "status": "present" if present else "missing", | |
| "description": artifact.description, | |
| } | |
| if present and absolute_path.is_file(): | |
| record["size_bytes"] = absolute_path.stat().st_size | |
| return record |
Comment on lines
+142
to
+154
| def determine_overall_status(local_records: Iterable[ArtifactRecord], optional_records: Iterable[ArtifactRecord]) -> str: | ||
| """Return release-readiness status from local and optional artifacts.""" | ||
| local_records = list(local_records) | ||
| optional_records = list(optional_records) | ||
| required_missing = [record for record in local_records if record["required"] and not record["present"]] | ||
| if required_missing: | ||
| return "red" | ||
|
|
||
| optional_missing = [record for record in optional_records if not record["present"]] | ||
| if optional_missing: | ||
| return "yellow" | ||
|
|
||
| return "green" |
Contributor
There was a problem hiding this comment.
The status determination logic can be made more robust by processing all records together and relying on the required attribute of each artifact. This approach handles potential future changes where local artifacts might be optional.
Suggested change
| def determine_overall_status(local_records: Iterable[ArtifactRecord], optional_records: Iterable[ArtifactRecord]) -> str: | |
| """Return release-readiness status from local and optional artifacts.""" | |
| local_records = list(local_records) | |
| optional_records = list(optional_records) | |
| required_missing = [record for record in local_records if record["required"] and not record["present"]] | |
| if required_missing: | |
| return "red" | |
| optional_missing = [record for record in optional_records if not record["present"]] | |
| if optional_missing: | |
| return "yellow" | |
| return "green" | |
| def determine_overall_status(records: Iterable[ArtifactRecord]) -> str: | |
| """Return release-readiness status from artifact records.""" | |
| records_list = list(records) | |
| if any(r["required"] and not r["present"] for r in records_list): | |
| return "red" | |
| if any(not r["present"] for r in records_list): | |
| return "yellow" | |
| return "green" |
Comment on lines
+160
to
+163
| optional_records = [artifact_record(artifact, REPORT_DIR) for artifact in OPTIONAL_CROSS_REPO_ARTIFACTS] | ||
| missing_required = [record["path"] for record in local_records if not record["present"]] | ||
| missing_optional = [record["path"] for record in optional_records if not record["present"]] | ||
| overall_status = determine_overall_status(local_records, optional_records) |
Contributor
There was a problem hiding this comment.
Update the artifact processing and status determination to use the simplified artifact_record and determine_overall_status functions.
Suggested change
| optional_records = [artifact_record(artifact, REPORT_DIR) for artifact in OPTIONAL_CROSS_REPO_ARTIFACTS] | |
| missing_required = [record["path"] for record in local_records if not record["present"]] | |
| missing_optional = [record["path"] for record in optional_records if not record["present"]] | |
| overall_status = determine_overall_status(local_records, optional_records) | |
| optional_records = [artifact_record(artifact) for artifact in OPTIONAL_CROSS_REPO_ARTIFACTS] | |
| missing_required = [record["path"] for record in local_records if not record["present"]] | |
| missing_optional = [record["path"] for record in optional_records if not record["present"]] | |
| overall_status = determine_overall_status(local_records + optional_records) |
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
1 participant
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.
Motivation
Description
scripts/generate_dashboard_health_summary.py, a Python 3.11+ stdlib-only generator that inspects report existence and small filesystem metadata and emits deterministic artifacts withgenerated_at: "2026-01-01T00:00:00Z"andsummary_type: "dashboard_release_health".docs/reports/dashboard-health-summary.mdanddocs/reports/dashboard-health-summary.jsoncontainingoverall_status(red/yellow/green), namedchecks,required_artifacts_present,missing_artifacts,next_recommended_actions, andsafety_notes(no secrets, synthetic data only)..github/workflows/agent-checks.ymlto compile and run the generator as part of the agent checks, and updated docsdocs/API_SURFACE.mdanddocs/AGENT_WORKFLOW.mdto reference the generator and how dashboards/UI can consume the JSON summary.red; required present but optional cross-repo artifacts missing =>yellow; all required and optional present =>green.Testing
python -m py_compile scripts/generate_dashboard_health_summary.py— passed.python scripts/generate_dashboard_health_summary.py— ran and produceddocs/reports/dashboard-health-summary.mdanddocs/reports/dashboard-health-summary.json(passed).python scripts/run_checks.py— executed successfully as part of validation steps after adding the generator (passed).Closes #29.
Codex Task