Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b737ef81f
ℹ️ 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".
| const config = await readScannerConfig(root) | ||
| if (!replacing && (config.icons ?? []).some(pack => pack.id === id)) throw new Error(`Icon pack already configured: ${id}`) | ||
| if (config.scanners.some(pack => pack.id === id) || config.workSources?.some(pack => pack.id === id)) throw new Error(`Plugin id already configured: ${id}`) | ||
| await writeScannerConfig(root, { ...config, icons: [...(config.icons ?? []).filter(pack => pack.id !== id), { id, source }] }) |
There was a problem hiding this comment.
When two configured packs expose the same bare icon name, loadIconCatalog intentionally lets the first pack win, but replacing a pack removes it and appends it to the end of the array. Running groma plugin update on the first pack therefore silently changes every unqualified reference such as database to the competing pack's artwork. Replace the entry in place so an update does not alter configured resolution order.
AGENTS.md reference: AGENTS.md:L265-L267
Useful? React with 👍 / 👎.
| ): SvgNode { | ||
| const layout = surfaceLabelLayout(text, size, zoom, view) | ||
| return node('g', { transform: planeMatrix('ground', text.origin, view) }, 'label surface-label', [ | ||
| ...(iconSvg ? [iconImage(iconSvg, layout.x - layout.fontSize * 1.2, layout.baseline - layout.fontSize * 0.9, layout.fontSize)] : []), |
There was a problem hiding this comment.
For system and container elements with short titles, this places the icon to the left of a title whose origin was already centered without accounting for the icon. Although placement adds three cells to the surface width, that space is split across both sides, while the icon is shifted left by roughly 1.2 font sizes; at larger adaptive label sizes it consequently extends beyond the reserved band, where it can overlap adjacent content or be cropped from an export. Center and size the combined icon-and-title group within the band to satisfy the map and cover rendering criterion.
AGENTS.md reference: AGENTS.md:L265-L267
Useful? React with 👍 / 👎.
| ...(element.icon === undefined ? {} : { icon: element.icon }), | ||
| ...(element.colour === undefined ? {} : { colour: element.colour }), |
There was a problem hiding this comment.
When a component changes only its icon or colour between revisions, compareArchitecture still marks it unchanged because its ownContent fingerprint does not include either newly propagated field. If another field changes at the same time, the component is marked modified but the comparison reasons and detail view still omit the appearance change. Include icon and colour in comparison detection and presentation so history reports these authored changes.
AGENTS.md reference: AGENTS.md:L265-L267
Useful? React with 👍 / 👎.
| if (config.useGitignore !== undefined && typeof config.useGitignore !== 'boolean') { | ||
| throw new Error(`${sourceFilename} useGitignore must be true or false`) | ||
| } | ||
| const icons = configuredIcons(config.icons, sourceFilename) |
There was a problem hiding this comment.
When a documented plugins.json configuration is edited directly, an icon pack can reuse a scanner or work-source ID because configuredIcons validates only duplicates within the icons array and the later scanner-ID set is not seeded from icons or work sources. ID-based operations then become ambiguous: removePlugin and updatePlugin select the icon entry first, while scanner restoration can become unreachable. Validate plugin IDs across all sections while parsing the configuration, not only on CLI additions.
AGENTS.md reference: AGENTS.md:L265-L267
Useful? React with 👍 / 👎.
| const style = classes.includes('text') ? `fill:${colour}` | ||
| : classes.includes('face') || classes.includes('ground') ? `stroke:${colour};fill:${accentTint(colour)}` : undefined |
There was a problem hiding this comment.
When an element has an authored colour, these inline fill and stroke declarations outrank all of the existing stylesheet rules for lifecycle and interaction state. A coloured draft therefore renders with solid faces instead of the required ghost outline, and selected, flow-lit, or comparison-marked elements retain their authored strokes and fills instead of the highlight or diff colours. Represent the accent in a way that the state rules can override, while retaining explicit values for static exports.
AGENTS.md reference: AGENTS.md:L318-L319
Useful? React with 👍 / 👎.
| icon?: string | ||
| colour?: string |
There was a problem hiding this comment.
If an otherwise empty scanned element has an authored icon or colour, groma edit <target> --combine <source> accepts it because requireCombinableSources checks only group and technology; the source document is then removed and both appearance fields are silently lost. Treat these new fields as authored metadata in the combinability guard, or define an explicit merge rule, just as the existing authored fields are protected.
AGENTS.md reference: AGENTS.md:L226-L229
Useful? React with 👍 / 👎.
Update the main package dependencies and use Bun 1.4.3 for development and all Bun workflows. Adapt Groma's terminal lifecycle to OpenTUI 0.6.0 so selecting a welcome action waits for terminal suspension, and closing a terminal view waits for restoration before streams are reused or the next welcome screen opens. | Package | Before | After | | --- | --- | --- | | `@opentui/core` | 0.5.12 | 0.6.0 | | `ignore` | 7.0.10 | 7.0.12 | | `@biomejs/biome` | 2.5.14 | 2.5.15 | | `@types/node` | 26.6.2 | 26.6.5 | | `typescript` | 7.1.0-dev.20260924.1 | 7.1.0-dev.20261011.1 | Welcome now awaits `suspend()` before exposing an action. Its CLI owner awaits session cleanup before reopening. The map returns OpenTUI's `renderer.closed` directly, removing its separate early completion promise. Standalone scanner settings and viewer setup failures also await terminal restoration. Scanner, example, and other workspace package files and versions are unchanged. Keep workspace links, the Parcel watcher patch, and the existing TypeScript compiler families. Root TypeScript uses the newest preview in its existing 7.1 family; the lockfile retains the scanner's previous compiler separately. Other root packages already match latest stable releases, and `@types/bun` 1.4.2 remains latest published. Release checks: [OpenTUI breaking changes](https://github.com/anomalyco/opentui/releases/tag/v0.6.0), [ignore patch changes](kaelzhang/node-ignore@7.0.10...7.0.12), [Biome 2.5.15](https://github.com/biomejs/biome/releases/tag/@biomejs%2Fbiome@2.5.15). Existing framebuffer and palette APIs continue to work; no unrelated Bun features or new architecture concepts are introduced. Validation on macOS arm64 with Bun 1.4.3: - Three concurrent lifecycle regressions failed before the fix and passed afterward: welcome suspension, welcome close, and map close must wait for their transitions. - `bun run check`: 16 Node tests and 779 Bun tests passed; 51 optional scanner/package tests skipped, zero failures. - `bun install --frozen-lockfile`: no changes. Standalone build passed. - Compiled `tui-test` checks passed: welcome → settings child → welcome → quit; map root → container → arrow selection → details → 200×60 resize → Ctrl+C. Screenshots were visually checked. - Scanner/example files remain unchanged; `git diff --check` passed and `groma/` is unchanged. Also pin the architecture comparison action to [groma.md-action PR #6](MrLesk/groma.md-action#6), commit `b5eb98ce51df144ba276182a5b80a7a2ce6d11a3`. Comparison preparation no longer requires `scanners.json`; export reads committed architecture and source from an empty temporary working root without loading project scanner hooks or changing configuration. The released scan path stays unchanged until the plugin CLI is released. Action validation: all 26 tests and both GitHub Check runs pass. Real exports pass with Groma 0.6.6 and the plugin-branch CLI, including the exact commits that previously failed in PR #118. Because comparison runs use `pull_request_target`, this pin takes effect after this PR is merged. Tracks TASK-579.
Groma architecture comparison
|
# Conflicts: # src/plugin-management.ts # src/scanner/modules/config.ts # src/scanner/modules/package.ts # src/scanner/modules/published.ts # src/scanner/modules/settings-model.ts # src/scanner/modules/settings.ts # src/viewers/web/scanners/settings.ts # test-bun/scanner-update.test.ts
# Conflicts: # groma/systems/groma-md/containers/cli/components/package.md # src/plugin-management.ts # src/scanner/modules/config.ts # src/scanner/modules/inventory.ts # src/scanner/modules/package.ts # src/scanner/modules/published.ts # src/scanner/modules/settings.ts
1b737ef to
aa470d5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa470d5789
ℹ️ 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".
| if (palette === undefined) { | ||
| return rules('.accented', 'currentColor', | ||
| `color-mix(in srgb, currentColor ${percent(ACCENT_TINT)}, var(--paper))`, | ||
| `color-mix(in srgb, var(--ink) ${percent(ACCENT_TITLE)}, currentColor)`) |
There was a problem hiding this comment.
For supported custom hex colours, the fixed 45% title mix does not guarantee the readability claimed by the appearance contract: in the light theme #ffffff produces a #9c9da1 title on a white surface (about 2.71:1 contrast), while in the dark theme #000000 produces about 3.46:1. The test covers only the curated named palette, so valid #RRGGBB input can make map and cover titles difficult to read; derive the title colour or mix ratio from the actual accent and theme.
AGENTS.md reference: AGENTS.md:L226-L229
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| export const elementTitleCss = ` | ||
| #details h1 .element-name { color: var(--element-colour, inherit); } |
There was a problem hiding this comment.
When a valid accent matches the current background, this raw colour makes the selected element's name disappear—for example, --colour '#ffffff' in the light theme or #000000 in the dark theme. Some named accents such as yellow also have weak contrast on the light details surface. Use a theme-aware readable derivative for heading text while retaining the authored colour for the swatch and map outline.
AGENTS.md reference: AGENTS.md:L226-L229
Useful? React with 👍 / 👎.
This branch was successfully deployed
1 participant
Implements TASK-561. Elements can carry an icon and an accent colour (
groma edit <id> --icon <name> --colour <name>and the web details form), stored asgroma.iconandgroma.colourand preserved by scans. Icons come from icon packs: packages that declare their SVGs undergroma.iconsand install withgroma plugin addinto theiconssection ofgroma/plugins.json. A name resolves through the installed packs or is an emoji, and an unknown name shows up ingroma lint.The map, the cover export and both details panes draw the icon and colour the same way in every theme. A legend lists the colours in use with a count each.
plugins/icons/architectureis a first pack of twelve generic icons, built and released like the scanner packages, anddocs/icons.mdexplains how to make and publish one.A Codex agent (GPT-6.1 Sol) built this live during the Devoxx Belgium keynote on 9 October, working TASK-561 in one checkout it shared with five other agents. This branch holds only this task's changes: I matched every changed line to the agent session that wrote it and rebuilt the task on a fresh branch from main.
Stacked on #117. The agent built icon packs on TASK-568's plugin system: plugin kinds,
plugins.jsonandgroma plugin add. The two share new files, so this PR shows only the icon work on top of that branch.Acceptance criteria, as the agent left them
Validation
On stage the agents were told not to install, build or run tests, so these are the first runs of this code.
bun run typecheckpasses on top of the Replace scanners.json with plugins.json and load plugins only from it #117 branch.biome lint: no errors. Three new warnings: cognitive complexity inedit.tsandpackage.ts, and one optional-chain hint.Overlap with the other PRs from this run
The criticality PR (#122) adds its field next to these in the element model, the emitter,
groma edit, the sheet, the building painter, the details panes and the component Markdown contract. Both PRs add separate fields side by side, so merging the second one means keeping both.render.tsis shared with the playback (#121) and metrics (#123) PRs, andwrite-commands.tswith the plans PR (#120).