diff --git a/AGENTS.md b/AGENTS.md index e9240f13cab..6a8fefb4efd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,6 +76,12 @@ - To run a subset of the tests: `ember test --path dist --filter "some text that appears in module name or test name"` Note that the filter is matched against the module name and test name, not the file name! Try to avoid using pipe characters in the filter, since they can confuse auto-approval tool use filters set up by the user. +- **Always capture test output to a file.** A host test run can produce hundreds of KB of output (browser logs, indexer warnings, per-test diagnostics). If you only pipe through `tail`/`grep`, you lose everything else and have to re-run — which is slow and the bug may not reproduce. Redirect the full run to a file, then grep that file for failures, browser logs around a specific test, etc. + ``` + pnpm exec ember test --path dist --filter "Foo" 2>&1 | tee /tmp/host-test-foo.log + grep -E "^(not )?ok |^# " /tmp/host-test-foo.log # summary + per-test status + grep -B2 -A40 "not ok 27" /tmp/host-test-foo.log # detail for a specific failure + ``` - run `pnpm lint` in this directory to lint changes made to this package - run `pnpm lint:fix` directly in this directory to apply fixes for lint failures made to this package that can be automatically fixed. - the host tests report this error: diff --git a/packages/base/cards-grid.gts b/packages/base/cards-grid.gts index c96caf781b0..e34779d5e91 100644 --- a/packages/base/cards-grid.gts +++ b/packages/base/cards-grid.gts @@ -1,7 +1,7 @@ import { registerDestructor } from '@ember/destroyable'; import { on } from '@ember/modifier'; import { action } from '@ember/object'; -import { tracked } from '@glimmer/tracking'; +import { cached, tracked } from '@glimmer/tracking'; import { restartableTask } from 'ember-concurrency'; import { modifier } from 'ember-modifier'; import { TrackedArray } from 'tracked-built-ins'; @@ -12,12 +12,15 @@ import { HighlightIcon } from '@cardstack/boxel-ui/icons'; import LayoutGridPlusIcon from '@cardstack/boxel-icons/layout-grid-plus'; import Captions from '@cardstack/boxel-icons/captions'; import AllCardsIcon from '@cardstack/boxel-icons/square-stack'; +import AllFilesIcon from '@cardstack/boxel-icons/files'; +import FileIcon from '@cardstack/boxel-icons/file'; import { cardIdToURL, chooseCard, specRef, baseRealm, + baseFileRef, isCardInstance, SupportedMimeType, subscribeToRealm, @@ -101,21 +104,26 @@ class Isolated extends Component { private cardTypeFilters: FilterOption[] = new TrackedArray(); + private fileTypeFilters: FilterOption[] = new TrackedArray(); private highlightsCards: BoxComponent[] = new TrackedArray(); - private filterOptions: FilterOption[] = []; private viewOptions: ViewOption[] = new TrackedArray([StripView, GridView]); private sortOptions: SortOption[] = new TrackedArray(SORT_OPTIONS); @tracked private activeViewId: ViewOption['id'] = this.viewOptions[1].id; @tracked private activeFilter!: FilterOption; @tracked private activeSort: SortOption = this.sortOptions[0]; + // Tracked separately from `fileTypeFilters.length` so the All Files + // group still appears when the only file summaries the realm has are + // bare `FileDef` rows (we exclude those from the leaf list to avoid + // duplicating the group's own root). Without this, a realm that only + // contains binary/unmapped files would silently hide the entire group. + @tracked private hasAnyFileSummary = false; #unsubscribeFromRealm: (() => void) | undefined; #subscribedRealm: string | undefined; constructor(owner: any, args: any) { super(owner, args); - this.setupFilterOptions(); this.activeFilter = this.filterOptions[0]; this.loadHighlightsCards.perform(); registerDestructor(this, () => this.teardownRealmSubscription()); @@ -148,6 +156,12 @@ class Isolated extends Component { return realmHref?.includes('/personal/') ?? false; } + // The per-group filter wrappers are `@cached` so their object identity + // stays stable across re-computations of `filterOptions`. `FilterList`'s + // `isSelected` is an identity comparison (`@filter === @activeFilter`), so + // returning a fresh object on every getter access would break the active + // highlight after the first render. + @cached private get highlightFilter(): FilterOption { return { displayName: 'Highlights', @@ -156,6 +170,7 @@ class Isolated extends Component { }; } + @cached private get allCardsFilter(): FilterOption { return { displayName: 'All Cards', @@ -174,12 +189,45 @@ class Isolated extends Component { }; } - private setupFilterOptions() { - this.filterOptions.splice(0, this.filterOptions.length); + @cached + private get allFilesFilter(): FilterOption { + return { + displayName: 'All Files', + icon: AllFilesIcon, + query: { + filter: { + type: baseFileRef, + }, + }, + filters: this.fileTypeFilters, + isExpanded: false, + }; + } + + // Derived from tracked state — `isPersonalRealm` and `hasAnyFileSummary` + // are the only deps that change the visible group set. We avoid the + // previous `splice + push` approach because mutating a `TrackedArray` + // during the component constructor (which runs mid-render) fires Glimmer's + // "you attempted to update X but it was already used in this computation" + // assertion. A `@cached` getter only re-runs when its tracked deps change + // and stays stable otherwise, so identity-based comparisons keep working. + @cached + private get filterOptions(): FilterOption[] { + let options: FilterOption[] = []; if (this.isPersonalRealm) { - this.filterOptions.push(this.highlightFilter); + options.push(this.highlightFilter); + } + options.push(this.allCardsFilter); + // Hide the All Files group only when the realm has no file rows at all + // — matches the "empty groups: hide" decision in the Linear plan. We + // can't use `fileTypeFilters.length > 0` here because bare `FileDef` + // rows are intentionally excluded from the leaf list (they would be a + // duplicate of the group itself), so a realm with only bare FileDef + // files would otherwise lose the entire group. + if (this.hasAnyFileSummary) { + options.push(this.allFilesFilter); } - this.filterOptions.push(this.allCardsFilter); + return options; } private teardownRealmSubscription() { @@ -293,23 +341,56 @@ class Isolated extends Component { displayName: string; total: number; iconHTML: string | null; + // Older realm-server builds may not stamp `kind` yet — treat missing + // as 'instance' so this client stays compatible during a rolling + // deploy. New servers always set the discriminator. + kind?: 'instance' | 'file'; }; }[]; let excludedCardTypeIds = [ `${baseRealm.url}card-api/CardDef`, `${baseRealm.url}cards-grid/CardsGrid`, ]; + // The "All Files" group already represents the bare FileDef root — listing + // it again as a leaf would just be a duplicate row. + let excludedFileTypeIds = [`${baseRealm.url}card-api/FileDef`]; this.cardTypeFilters.splice(0, this.cardTypeFilters.length); + this.fileTypeFilters.splice(0, this.fileTypeFilters.length); + let sawFileSummary = false; cardTypeSummaries.forEach((summary) => { - if (!summary.id || excludedCardTypeIds.includes(summary.id)) { + if (!summary.id) { return; } let codeRef = codeRefFromInternalKey(summary.id); if (!codeRef) { return; } + let kind = summary.attributes.kind ?? 'instance'; + if (kind === 'file') { + // Even when the only file summary is the bare-FileDef root (which + // we exclude from the leaf list), we still want the All Files + // group to appear in the sidebar — otherwise the user has no way + // to reach those files. Flip this flag before the leaf-list gate. + sawFileSummary = true; + if (excludedFileTypeIds.includes(summary.id)) { + return; + } + this.fileTypeFilters.push({ + displayName: summary.attributes.displayName ?? codeRef.name, + icon: summary.attributes.iconHTML ?? FileIcon, + query: { + filter: { + type: codeRef, + }, + }, + }); + return; + } + if (excludedCardTypeIds.includes(summary.id)) { + return; + } this.cardTypeFilters.push({ displayName: summary.attributes.displayName ?? codeRef.name, icon: summary.attributes.iconHTML ?? Captions, @@ -321,6 +402,13 @@ class Isolated extends Component { }); }); + this.hasAnyFileSummary = sawFileSummary; + + // `filterOptions` is a @cached getter that derives the group list from + // tracked state — `fileTypeFilters.length` changing here causes it to + // re-compute on the next read, which adds/removes the All Files group + // without any imperative array mutation. + let flattenedFilters: FilterOption[] = []; this.filterOptions.map((f) => f.filters?.length diff --git a/packages/base/components/card-list.gts b/packages/base/components/card-list.gts index 99dbcc0fa9b..5b07294314c 100644 --- a/packages/base/components/card-list.gts +++ b/packages/base/components/card-list.gts @@ -7,6 +7,8 @@ import { consume } from 'ember-provide-consume-context'; import { LoadingIndicator } from '@cardstack/boxel-ui/components'; +import FileIcon from '@cardstack/boxel-icons/file'; + import { cn, eq } from '@cardstack/boxel-ui/helpers'; import { @@ -48,6 +50,33 @@ export default class CardList extends Component { } } + // Last URL segment, used as a visible label when the prerender pipeline + // didn't produce HTML for a file row (CS-11171 — `.gts`/`.ts` FileDef rows + // currently skip the FileRender pass, so their `fitted_html` is null and + // `` renders nothing). + fileNameFromUrl(url: string): string { + try { + let pathname = new URL(url).pathname; + let segment = pathname.split('/').filter(Boolean).pop(); + return segment ?? url; + } catch { + let segments = url.split('/').filter(Boolean); + return segments[segments.length - 1] ?? url; + } + } + + // Render the filename fallback only when the prerender pipeline produced + // no HTML AND the row is not an error. Error rows have their own + // dedicated error component built by PrerenderedCard's constructor, which + // also has empty `html`; treating them like file fallbacks would swap a + // helpful "rendering error" affordance for a bare filename. + shouldRenderFallback(card: { + hasHtml?: boolean; + isError?: boolean; + }): boolean { + return card.hasHtml === false && !card.isError; + } +