Skip to content

refactor(reports): read validation artifacts asynchronously - #62

Merged
phodal merged 1 commit into
QoderAI:mainfrom
dxiaoqi:refactor/async-report-validation-reads
Aug 4, 2026
Merged

refactor(reports): read validation artifacts asynchronously#62
phodal merged 1 commit into
QoderAI:mainfrom
dxiaoqi:refactor/async-report-validation-reads

Conversation

@dxiaoqi

@dxiaoqi dxiaoqi commented Aug 4, 2026

Copy link
Copy Markdown
## Summary

Read generated findings, Markdown, and optional HTML artifacts concurrently
during report validation. This removes synchronous filesystem reads from the
asynchronous validation pipeline without changing report output or validation
decisions.

## Why

- Issue/Story: None — justified maintenance optimization; no existing issue was
  found for this change.
- User or maintainer outcome: Report validation no longer blocks the Node.js
  event loop while loading independent generated artifacts.

## Traceability and Scope

- Spec/ADR, if applicable:
  `docs/specs/2026-08-04-async-report-validation-reads.md`
- Acceptance criteria addressed: ARV-AC-1, ARV-AC-2, ARV-AC-3, ARV-AC-4
- Canonical owners changed:
  `scripts/harness-analysis/render-report.mjs`
- Explicit non-goals:
  - No report schema or validation-rule changes.
  - No reader-visible output changes.
  - No CLI, host-support, or run-directory changes.
  - No parallelization of artifact writes, Canvas validation, publication, or
    rollback.
  - No dependency or generated-file changes.

## Change Type

- [ ] Feature
- [ ] Bug fix
- [ ] Tests only
- [ ] Documentation/community
- [x] Refactor with no intended behavior change
- [ ] Dependency, packaging, or infrastructure

## Test and Review Evidence

| Check | Result |
| --- | --- |
| `node --test --test-name-pattern="render command writes disk-openable HTML artifacts\|HTML mode validates canonical compact" test/harness-report-render-cli.test.mjs` | Passed 2/2 |
| `node --test test/harness-report-render-cli.test.mjs` | Passed 32/32 |
| `npm test` | Ran 1,148 tests: 1,145 passed; three preview tests were blocked by sandbox-only `listen EPERM` |
| `node --test test/preview-servers.test.mjs` with loopback permission | Passed 8/8 |
| `node scripts/doc-link-graph/cli.mjs skills/better-harness` | Generated the current graph with 35 files and 51 links; no generated diff |
| `node --test test/doc-link-graph.test.mjs` | Passed 6/6 |
| `env npm_config_cache=/tmp/better-harness-npm-cache-20260804 npm run pack:verify` | Passed with 395 npm entries and 417 runtime ZIP entries |
| `git diff --check` | Passed |

Manual or visual evidence:

The implementation, Spec, and final diff were manually reviewed. No visual
behavior changed, so visual screenshots were not applicable.

Control-flow review confirmed that:

- Generated files are still written to the staging directory.
- Artifact reads and validation occur before publication.
- Failed validation still prevents the staged run from being published.
- `publishStagedRun` remains behind the validation gate.
- Renderer artifact names and result ordering remain unchanged.

## Risk and Recovery

- Compatibility and cross-platform impact: The implementation uses the standard
  `node:fs/promises` API supported by the repository's declared Node versions
  on Windows, macOS, and Linux. No platform-specific shell behavior was added.
  Cross-platform CI results remain pending.
- Package, plugin, schema, or generated-file impact: One shipped renderer script
  changed. Package verification passed. No dependencies, schemas, plugin
  metadata, or generated files changed.
- Rollback or recovery path: Revert commit `2ada64e`.
- Residual risk or unverified boundary: If multiple required staging artifacts
  become unreadable simultaneously, `Promise.all` may change which filesystem
  error is observed first. The staging and atomic publication boundaries remain
  unchanged. Local validation was performed before synchronizing the branch
  with the latest `main`.

## AI Involvement

- Level: Generated
- Human review and validation: The contributor reviewed the implementation,
  Spec, final diff, and recorded validation results.

## Checklist

- [x] I followed `AGENTS.md`, `CONTRIBUTING.md`, and the relevant canonical-owner guidance.
- [x] The change is focused and does not include unrelated local or generated state.
- [x] Tests and documentation match the behavior actually delivered.
- [x] Markdown links were checked when documentation moved or changed.
- [x] Cross-platform behavior was considered for Windows, macOS, and Linux.
- [x] Package/runtime verification was run when shipped files or dependencies changed.
- [x] User-facing or compatibility changes are recorded in `CHANGELOG.md`. <!-- N/A: this refactor has no user-facing or compatibility change. -->
- [x] I have the right to contribute this work under the repository's MIT License.

Implements asynchronous report-validation reads using docs/specs/2026-08-04-async-report-validation-reads.md. Independent generated artifacts now load concurrently while preserving validation order and staged publication.

Validated with the focused HTML checks, the 32-test render suite, 6 documentation-link checks, 8 loopback preview checks, and isolated-cache package verification. The full suite passed 1,145 of 1,148 tests; the remaining three were sandbox-only loopback EPERM failures.

Co-authored-by: Codex (GPT 5.6 Sol) <codex@openai.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors report validation to read generated artifacts (findings JSON, Markdown report, and optional HTML report) concurrently using node:fs/promises, removing synchronous filesystem reads from the async validation flow while preserving validation behavior and output.

Changes:

  • Replace readFileSync usage with readFile from node:fs/promises in render-report.mjs.
  • Read findings/Markdown/optional-HTML in a single Promise.all during validation.
  • Add a spec documenting intent, acceptance scenarios, risk note, and test evidence.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
scripts/harness-analysis/render-report.mjs Switches validation artifact loading to concurrent async reads and reuses the loaded HTML text for HTML validation.
docs/specs/2026-08-04-async-report-validation-reads.md Documents the refactor’s intent, ACs (ARV-AC-1..4), plan, and observed test/review evidence.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@phodal
phodal merged commit 4cd2fdb into QoderAI:main Aug 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants