Improve software-factory validation/result cards#5363
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 721ebf78c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4a14080 to
8349e20
Compare
There was a problem hiding this comment.
Pull request overview
This PR reworks the software-factory validation/result card UI for more consistent fitted vs isolated rendering, tightens the template-lint guardrails around data-test-* selectors (including in <style> blocks), and improves bootstrap wiring behavior in the factory runtime.
Changes:
- Refactors validation/result cards (
parse-result,lint-result,eval-result,instantiate-result,test-results) to use sharedResultFittedCard/ResultIsolatedCardpresentation components and updated status/meta rendering. - Updates Overview’s “Validation Runs” to group by validation type (pipeline order) and query newest-first, plus small icon sizing tweaks.
- Tightens the
no-data-test-selectorrule to scan concatenated<style>block child text; simplifies realm bootstrap wiring (shareddelay(), concurrent link wiring, deterministic newest-first linking via sort).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/template-lint/lib/no-data-test-selector.mjs | Expands scanning to catch [data-test- in <style> blocks even when split across nodes / concatenations. |
| packages/software-factory/src/realm-operations.ts | Reuses delay() from runtime-common and simplifies relationship linking to always choose the first search result. |
| packages/software-factory/src/factory-realm-index.ts | Makes board linking deterministic by sorting newest-first instead of custom ID selection logic. |
| packages/software-factory/src/factory-entrypoint.ts | Defaults bootstrap wiring to retry for index readiness and runs board/project linking concurrently. |
| packages/software-factory/realm/result-fitted-card.gts | New shared fitted-card UI component for consistent result-card rendering. |
| packages/software-factory/realm/result-isolated-card.gts | New shared isolated-card UI shell used by result card “full view” layouts. |
| packages/software-factory/realm/test-results.gts | Refactors test run cards to shared result-card components and richer per-test status rendering. |
| packages/software-factory/realm/parse-result.gts | Refactors parse result cards to shared result-card components with updated meta/status rendering. |
| packages/software-factory/realm/lint-result.gts | Refactors lint result cards to shared result-card components and improved violation presentation. |
| packages/software-factory/realm/instantiate-result.gts | Refactors instantiate result cards to shared result-card components and updated error presentation. |
| packages/software-factory/realm/eval-result.gts | Refactors eval result cards to shared result-card components and updated error presentation. |
| packages/software-factory/realm/overview.gts | Switches validation runs to per-type grouping + runAt sorting; removes local Option redeclare; adds explicit icon sizing. |
| .agents/skills/boxel-development/references/dev-enumerations.md | Updates documentation example to use cardTitle instead of title. |
| .agents/skills/boxel-development/references/dev-core-patterns.md | Updates docs to emphasize cardTitle as the host-read display name field and corrects related examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The local delay() in realm-operations.ts duplicated the identical helper already exported from @cardstack/runtime-common. Import the shared one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…of redeclaring overview.gts redeclared a local Option interface identical to the one already exported by kanban-config.gts (which it imports from). Use the shared type so it can't drift from the option arrays it describes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Five boxel-icons in overview.gts were sized only through CSS classes, leaving the prerenderer without an intrinsic size. Move the dimensions onto explicit width/height attributes (the convention the file's Rocket icon already follows); CSS classes keep only color.
The onBootstrapComplete hook searched for the freshly-pushed board/Project with zero retries, but those cards are synced fire-and-forget (no waitForIndex), so the search usually raced the indexer and the hook no-opped. For runs that stall or are interrupted before the post-loop backstop runs, that hook is the only wiring — so losing the race left the dashboard with no board and the seed issue with no project link, exactly what the hook exists to prevent. Default the hook to waitForIndex too so both paths poll until the cards are indexed.
linkBoardToRealmIndex picked the lexicographically-first IssueTracker id while linkProjectToSeedIssue picked the most-recently-modified Project. If a re-run ever produced more than one of each, the dashboard board and the seed issue's project could resolve to different generations of bootstrap output. Sort both searches newest-first and take the first result, and drop the now-unused selectId hook from linkRelationshipToCard.
wireBootstrapArtifacts awaited linkBoard then linkSeedProject in series. Each runs its search through searchUntilNonEmpty, so on a realm that has neither card yet both burn their full retry budget back-to-back (~10s on the backstop path). The two links patch independent files and share no state, so run them with Promise.all and sync once if either changed — halving the worst-case wait.
…style>
The no-data-test-selector rule tested each <style> child TextNode in
isolation, so a [data-test-*] selector split across nodes by a comment or
{{! }}, or assembled inside a ConcatStatement, slipped past. Concatenate
the static text of all children before scanning. A fully dynamic body
(e.g. {{this.css}}) still can't be inspected, which the comment notes.
…equence sequenceNumber restarts at 1 per issue slug, so sorting a whole type group by it could rank an older issue's run ahead of a newer issue's. Sort by the global runAt timestamp so the Validation Runs widget is actually newest-first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Importing delay as a value from the @cardstack/runtime-common barrel pulls index-writer.ts into the node test transpile pipeline, where its `declare private` field trips a babel plugin-ordering SyntaxError. The /utils subpath has only type imports, so it loads no runtime modules. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…odes
collectStaticText concatenated TextNodes across MustacheStatements as if
they were absent, so `[data-` {{kind}} `test-` fused into `[data-test-`
and false-positived. Insert a delimiter for dynamic nodes; keep comments
transparent so a genuinely comment-split selector is still caught.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Validation Runs widget now renders one group per issue (each showing all five result types for that issue, newest run first) rather than one group per validation type. Drop the now-unused VALIDATION_TYPE_GROUPS list, the per-type query/getter, and the orphaned .validation-type style. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> fix(software-factory): sort validation runs by a general field Sorting the per-issue validation query by `runAt` returned nothing: a sort on a per-type card field requires an `on` CodeRef, which can't span the five result types in the `any` filter, so the query was invalid. Sort by `createdAt` (a general, type-agnostic field) instead — each result card is created when its run completes, so newest-created is newest-run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Revert the Validation Runs widget to one group per validation type (parse → lint → eval → instantiate → test), each sorted newest-first by runAt with an on CodeRef. Restores VALIDATION_TYPE_GROUPS, the per-type query, and the .validation-type style. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aaf352f to
f4e21d5
Compare
Summary
Reworks the software-factory's validation/result cards. Also tightens the
data-test-*lint rulesthat keep test-only hooks out of functional selectors.
What changed
Validation result cards
eval-result,lint-result,parse-result,test-results,instantiate-result— for clearer pass/fail presentation.result-fitted-cardandresult-isolated-cardcomponents so the result cards render consistently across fitted and isolated views, plus anempty-statecomponent for the no-results case.Lint enforcement for
data-test-*selectorsno-data-test-selectorrule now scans<style>blocks and catches selectors split or concatenated across text nodes /ConcatStatements.Misc cleanups
delay()from runtime-common and importedOptionfromkanban-config instead of redeclaring.
width/heightattributes.