fix(linear): rename the agent:* label family to ai:* in AGENTS.md - #17
fix(linear): rename the agent:* label family to ai:* in AGENTS.md#17Oba-One wants to merge 2 commits into
Conversation
Call extraction from the Jul 16 website audit plus a code and live-browser audit of the homepage map: dvh-driven sizing root cause, people-only clustering gaps, filter dead-ends, stale hover/selection state, and entry naming, with a prioritized P0-P2 plan and evidence index. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes PRD-756. Every routine spec and agent-facing doc instructed writers to apply `agent:routine` / `agent:codex` / `agent:claude`. No `agent` label group exists in Linear. The live group is `ai`, with exactly those three children. `save_issue` rejects the entire label array when any one entry fails to resolve, so an agent following the docs literally got a hard write failure and filed nothing, which is how health-watch hit this while filing PRD-755. Two traps this rename had to avoid: - `package:agent` is a real, unrelated label (the agent runtime package) and is untouched. This was not a find-and-replace of the word "agent". - `agent:copilot` appeared in the operating model as an example, and has no `ai:` equivalent at all: the live group is only routine, codex, and claude. Dropped rather than translated. Also documents the second failure mode from the same issue: the `group:child` notation used throughout these docs is display shorthand, not accepted API input. `save_issue` resolves by bare child name or ID, so `["protocol:green-goods", "activity:qa"]` fails where `["green-goods", "qa"]` succeeds. PRD-756`s own label array reads `[routine, qa, docs, green-goods]`, which is the proof. Stated at the canonical definition and at both routine-facing indexes, since that is where writers look. One further correction found while reading: the guild routines README said to "optionally add `ai:claude` or `ai:codex`" on top of `ai:routine`. Linear enforces one child per grouped family, so that write would be rejected for the same reason. It now says they replace rather than accompany. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a HomeMap UX audit document with findings, an implementation plan, and evidence references, while updating Codex Linear label examples from ChangesLinear label contract
HomeMap polish audit
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Superseded by #18. This branch was cut from a local |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
docs/design/map-polish-audit.md (1)
43-43: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDefine the anchor for people-only clusters.
A chapter-anchored cluster cannot represent a cluster containing only members or stewards. Specify a deterministic fallback, such as the centroid or highest-priority node, and define how labels and theme rings behave when no chapter is present.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/map-polish-audit.md` at line 43, Update the cluster rendering and interaction specification to define a deterministic anchor for people-only clusters, such as the centroid or highest-priority member/steward. Also specify the label content and theme-ring behavior when no chapter is present, while preserving chapter anchoring whenever a chapter exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/map-polish-audit.md`:
- Line 30: Update the hero sizing finding in the map-polish audit to replace the
typo “dek” with “deck,” without changing the surrounding content.
- Line 41: Update the overlap-stack description to replace “bit-identical
coordinates” with “same rounded coordinate key,” explicitly reflecting that
groupPins uses toFixed(2) and may group distinct points or separate nearby
boundary points.
- Line 76: Revise the A11y hygiene entry to replace the absolute “dropped by AT”
claim with “may not be exposed” or equivalent. Tie the recommendation to the
legend/controls wrapper semantics, retaining the suggestion to use appropriate
roles such as toolbar or group.
- Line 47: Revise the duplicate-identity guidance in the map audit so nodes are
not deduplicated by identity alone. Update the injection logic around the
referenced 3302-3308 flow to confirm source-record and affiliation semantics
before merging, while preserving distinct node IDs for legitimate records and
flagging or merging only confirmed duplicate source records; apply the same
clarification to the related guidance at the second occurrence.
- Around line 65-66: Update the P0 zero-result-state documentation to
distinguish true zero matches from matches that exist but are clipped, hidden,
or occluded. Replace the proposed “2 stewards match” empty-state wording with
separate diagnostics: one for no matching nodes and another indicating matching
nodes are not visible, while retaining the persistent “Show all” reset behavior
for active filters.
- Line 26: Update the sizing diagnosis in the map-polish audit to distinguish
the 200×100 projection space from the 88-unit map viewport; describe the
projection as 200×100 rendered into the 88-unit viewport, and remove the “200×88
world” wording unless the SVG viewBox verifies it.
---
Nitpick comments:
In `@docs/design/map-polish-audit.md`:
- Line 43: Update the cluster rendering and interaction specification to define
a deterministic anchor for people-only clusters, such as the centroid or
highest-priority member/steward. Also specify the label content and theme-ring
behavior when no chapter is present, while preserving chapter anchoring whenever
a chapter exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 189b670c-6095-401b-aa58-bcb1727824ef
📒 Files selected for processing (2)
AGENTS.mddocs/design/map-polish-audit.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/{package.json,bun.lockb,bun.lock,bunfig.toml,.github/workflows/**,AGENTS.md,CLAUDE.md,.codex/**,.claude/**}
📄 CodeRabbit inference engine (CLAUDE.md)
Treat package manifests, lockfiles, package-manager configuration, workflows, agent instructions, and Claude/Codex configuration as security-sensitive; call out changes to them in final summaries.
Files:
AGENTS.md
{package.json,bun.lock,bunfig.toml,.github/workflows/**,AGENTS.md,CLAUDE.md,.codex/**,.claude/**}
📄 CodeRabbit inference engine (AGENTS.md)
Treat package manifests, lockfiles, package-manager configuration, CI workflows, agent instructions, and assistant configuration as security-sensitive surfaces; call out changes to them in final summaries.
Files:
AGENTS.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: greenpill-dev-guild/network
Timestamp: 2026-07-27T20:03:37.109Z
Learning: Keep Codex defaults at `sandbox_mode = "workspace-write"`, `approval_policy = "on-request"`, `approvals_reviewer = "auto_review"`, and the `balanced` permissions profile; do not add `danger-full-access` or `never` overrides without explicit approval.
Learnt from: CR
Repo: greenpill-dev-guild/network
Timestamp: 2026-07-27T20:03:37.109Z
Learning: Use Linear as the durable backlog; never open GitHub Issues for backlog work.
Learnt from: CR
Repo: greenpill-dev-guild/network
Timestamp: 2026-07-27T20:03:37.109Z
Learning: Implement Linear-dispatched work only when the issue has acceptance criteria, a named package surface, and validation; otherwise stop and comment rather than guessing.
Learnt from: CR
Repo: greenpill-dev-guild/network
Timestamp: 2026-07-27T20:03:37.109Z
Learning: Act as executor rather than orchestrator: implement only the scoped unit and do not expand beyond acceptance criteria or make sequencing decisions for the human.
Learnt from: CR
Repo: greenpill-dev-guild/network
Timestamp: 2026-07-27T20:03:37.109Z
Learning: Work on the integration branch named in the issue, link the issue from the PR, keep one issue per PR, and never self-merge.
Learnt from: CR
Repo: greenpill-dev-guild/network
Timestamp: 2026-07-27T20:03:37.109Z
Learning: Before creating a PR, run matching type checks, focused tests, `bun run agentic:check` for website/UI work, and `bun run build`, and provide actual validation evidence.
🪛 LanguageTool
docs/design/map-polish-audit.md
[grammar] ~30-~30: Ensure spelling is correct
Context: ... short viewports.** At 641px height the dek is visible but both CTAs sit half-clipp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~30-~30: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...aller H1 margin, map height cap) so the full stack fits ~600px-tall laptops. ### 2.2 Node...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~42-~42: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tune77**, not any of the four chapters. Chapters are effectively unclickable in dense re...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~75-~75: Ensure spelling is correct
Context: ...ers only — live layer unavailable") and backoff. - P2 · A11y hygiene: legend/contro...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (2)
docs/design/map-polish-audit.md (1)
3-3: 🎯 Functional CorrectnessDo not infer deployment lag from node-count differences alone.
The production/local count delta does not establish that production is “at least one content build behind”; it could reflect intentional dataset or publication differences. Phrase this as an observed divergence unless deployment provenance confirms the lag.
AGENTS.md (1)
54-54: LGTM!
|
|
||
| ### 2.1 Sizing & first impression — the ThinkPad bug, explained | ||
|
|
||
| **P0 · The map's width is driven by viewport *height*.** `index.astro:385-388` sets the hero map wrapper to `width: min(100%, 128dvh)`. On 1366×641 (measured live): wrapper = **820px in a 1366px viewport** — 40% of the width sits empty beside the flagship element. Worse, the canvas flips to a **4/3 mobile aspect at ≤720px container width** (`HomeMap.astro:6743-6747`), and `128dvh` crosses that threshold whenever the viewport is shorter than ~563px — a ThinkPad at 125% OS scaling lands there. Then `preserveAspectRatio="xMidYMid meet"` letterboxes the 200×88 world inside the 4/3 canvas: **~100px of empty green above and below the map**. That is Matt's "skew." Side effects of the accidental mobile flip on a desktop machine: zoom controls hidden (`6890`), hover card disabled (`6825`), wheel/scroll captured by `touch-action:none` (`6749`). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the projection dimensions in the sizing diagnosis.
HomeMap.astro defines VIEW_W = 200, VIEW_H = 100, and MAP_VIEW_H = 88; “200×88 world” conflates the projection space with the map viewport. Describe the 200×100 projection rendered into the 88-unit map viewport, or verify the SVG viewBox before retaining this claim.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/map-polish-audit.md` at line 26, Update the sizing diagnosis in
the map-polish audit to distinguish the 200×100 projection space from the
88-unit map viewport; describe the projection as 200×100 rendered into the
88-unit viewport, and remove the “200×88 world” wording unless the SVG viewBox
verifies it.
|
|
||
| **P0 · Fix direction:** make the map's size a function of **container width** (with a height *cap*, not a height *driver*), and key the mobile flip off container width only — e.g. `width: min(100%, 1280px)`, `max-block-size: min(72svh, …)`, keep 200/88 aspect at all desktop widths. Opening devtools or a bookmarks bar should never change the map's layout mode. | ||
|
|
||
| **P1 · Hero doesn't fit short viewports.** At 641px height the dek is visible but both CTAs sit half-clipped below the fold. The hero stack (H1 → map → dek → 2 CTAs) needs a short-viewport budget (smaller H1 margin, map height cap) so the full stack fits ~600px-tall laptops. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the “dek” typo.
Use “deck” in the hero sizing finding.
🧰 Tools
🪛 LanguageTool
[grammar] ~30-~30: Ensure spelling is correct
Context: ... short viewports.** At 641px height the dek is visible but both CTAs sit half-clipp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~30-~30: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...aller H1 margin, map height cap) so the full stack fits ~600px-tall laptops. ### 2.2 Node...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/map-polish-audit.md` at line 30, Update the hero sizing finding
in the map-polish audit to replace the typo “dek” with “deck,” without changing
the surrounding content.
Source: Linters/SAST tools
| - The Great Lakes corridor is one blob: Ottawa/Toronto/London-Ontario/NYC are 4–17px apart *as chapters alone*, before members land on top. | ||
|
|
||
| **Why, in code:** | ||
| - The overlap-stack mechanism only groups **people**, and only at **bit-identical coordinates** (`groupPins` keys on `toFixed(2)` — `HomeMap.astro:2782, 2796-2803`). Two members 0.5px apart never cluster. Chapters are excluded entirely (different SVG group, `visiblePeoplePins` queries members only, `2784-2793`). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not describe toFixed(2) grouping as bit-identical.
The key groups coordinates by their rounded hundredth-unit string, not by exact equality. Distinct points can share a rounded key, while points near a rounding boundary can fall into different keys; call this “same rounded coordinate key” instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/map-polish-audit.md` at line 41, Update the overlap-stack
description to replace “bit-identical coordinates” with “same rounded coordinate
key,” explicitly reflecting that groupPins uses toFixed(2) and may group
distinct points or separate nearby boundary points.
|
|
||
| **P1 · Edge cases found live:** | ||
| - **Clipped node:** member "Amio" renders half outside the canvas edge (far-west longitude). Clamp x into the drawable inset or wrap threads/nodes; never clip a pin. | ||
| - **Duplicate identity:** "kitblake" appears at two locations (Amsterdam ~Germany, and at Emmanuel Jacobson's exact coords in West Africa). Dedup by identity at injection (`3302-3308` exists but missed this) and flag same-name/same-source nodes. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not deduplicate nodes by identity alone.
The same person may legitimately have multiple node records or locations. Deduplicate only after confirming source-record and affiliation semantics; otherwise this plan can silently remove valid map data. Prefer flagging or merging duplicate source records while preserving distinct node IDs.
Also applies to: 94-94
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/map-polish-audit.md` at line 47, Revise the duplicate-identity
guidance in the map audit so nodes are not deduplicated by identity alone.
Update the injection logic around the referenced 3302-3308 flow to confirm
source-record and affiliation semantics before merging, while preserving
distinct node IDs for legitimate records and flagging or merging only confirmed
duplicate source records; apply the same clarification to the related guidance
at the second occurrence.
| - **P0 · Zero-result states are blank, and reset is invisible (C3):** clicking **Stewards (2)** rendered an *empty world* — one steward is the clipped Amio node, the other wasn't visible — with no message and no way back except knowing to re-click the pill. Add (a) an on-canvas empty/sparse state ("2 stewards match — clear filters"), and (b) a persistent **"Show all"** reset chip whenever any filter is active. | ||
| - **P0 · Theme filters exempt chapters (C4):** `HomeMap.astro:2177-2182` hard-returns `true` for chapters — necessarily, because chapter nodes ship `themes: []` (`120`, `231`). Result observed on the call: filter by theme → members vanish, every chapter stays, counts don't move. Either (short-term) *dim* non-applicable chapters and label the legend "themes filter people," or (right fix) theme-tag chapters in content so the filter applies to everything. Chapter theme data is a content task, not a code task. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Separate “no matches” from “matches are not visible.”
The observation says two stewards match, but the canvas is blank because nodes are clipped or not visible. A “2 stewards match” empty-state message would misdiagnose a rendering/visibility failure as a zero-result state. Document distinct diagnostics for zero matches versus clipped, hidden, or occluded matches.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/map-polish-audit.md` around lines 65 - 66, Update the P0
zero-result-state documentation to distinguish true zero matches from matches
that exist but are clipped, hidden, or occluded. Replace the proposed “2
stewards match” empty-state wording with separate diagnostics: one for no
matching nodes and another indicating matching nodes are not visible, while
retaining the persistent “Show all” reset behavior for active filters.
| - **P0 · The primary CTA misleads (C5 inverted):** the hero's lime **"Find your people"** button opens the **add-your-node walkthrough** ("What are you here to grow?… *Already added yourself?*"). Discovery label → submission flow: the exact confusion Matt hit, now on the page's main action. With the locked rename: the walkthrough's trigger should be **"Join the map"** (on-map pill + footer), while "Find your people" should do what it says — scroll to/focus the map, or open the List/browse surface. | ||
| - **P2 · No on-map join affordance:** the only walkthrough trigger sits below the map (off-screen while the map is in view on mobile). Add the "Join the map" pill to the map surface itself. | ||
| - **P2 · Live layer is silent:** `/map/state` failure is a silent no-op (`3317-3320` — `DESIGN.md:642` says it should error the map); Stewards/Members read "0" indistinguishably from "down"; there's no loading state; a hard-down agent is re-polled every 5s forever. Add a quiet status line ("live · updated 2m ago" / "showing chapters only — live layer unavailable") and backoff. | ||
| - **P2 · A11y hygiene:** legend/controls wrappers are bare `<div aria-label>` without roles (dropped by AT) — use `role="toolbar"`/`group`; chapter anchors `preventDefault` primary click (inspector) while modified clicks navigate — intentional but should be documented; keep the one-shot thread animation (no idle motion) as is. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the assistive-technology claim.
A bare <div aria-label> may be ignored or inconsistently exposed without a semantic role; “dropped by AT” is too absolute. Use “may not be exposed” and tie the recommendation to the actual wrapper semantics.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/map-polish-audit.md` at line 76, Revise the A11y hygiene entry to
replace the absolute “dropped by AT” claim with “may not be exposed” or
equivalent. Tie the recommendation to the legend/controls wrapper semantics,
retaining the suggestion to use appropriate roles such as toolbar or group.
![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: 104680c660
ℹ️ 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".
| ## Linear-Spawned Issue Contract | ||
|
|
||
| Linear (workspace `greenpill-dev-guild`; teams Product `PRD` / Research `RESR`) is the durable backlog — never open GitHub Issues for backlog work. Codex work is labeled `agent:codex` (distinct from `agent:claude` interactive and `agent:routine` cron writes). The Linear MCP is wired into the Codex environment for read/query and state transitions. | ||
| Linear (workspace `greenpill-dev-guild`; teams Product `PRD` / Research `RESR`) is the durable backlog — never open GitHub Issues for backlog work. Codex work is labeled `ai:codex` (distinct from `ai:claude` interactive and `ai:routine` cron writes). The Linear MCP is wired into the Codex environment for read/query and state transitions. |
There was a problem hiding this comment.
This still presents ai:codex (and the other ai:* variants) as the literal label value, but the commit description establishes that group:child is display-only and save_issue resolves only a bare child name or ID. An agent following this canonical instruction will therefore still submit an unresolvable label and have the entire Linear write rejected—the failure this change is intended to fix. State the accepted values such as codex, or explicitly mark ai:* as display shorthand.
Useful? React with 👍 / 👎.
| - **P0 · Hover card gets stuck:** the kitblake card persisted through clicks, filter changes, opening the Themes panel, opening the List modal, and page scroll — stale UI layered over everything (canvas held `is-focused`). Clear focus state on pointer-leave, scroll, filter change, and any overlay opening. Same for **selection threads**: the teal/orange strands from a previous selection survived closing the inspector and even viewport changes. | ||
| - **P1 · Selected inspector is a center modal:** selecting a node opens a large card over the middle of the map — you lose the geography (where is this node?), it covers the legend, and its connections list clips mid-row with chips overflowing the card edge. Dock it to a side rail (desktop) so the selected node + its highlighted threads stay visible; fix the internal overflow. (Mobile's bottom-sheet pattern is the right idea — desktop should get the equivalent side dock.) Also: the closed inspector leaves 0×0 but still-`aria-pressed` connection buttons in the DOM — remove or `inert` them. | ||
| - **P1 · Themes panel is a wall:** it opens as a ~70%-of-map translucent panel (16 rows), unusable as a *legend* and it z-fights with any stuck hover card. Replace with a compact single-row chip strip (+`+N` overflow) for glancing, expanding to the full panel only for editing — with **Clear / All** actions (§2.4). | ||
| - **P1 · List is a blocking modal:** `showModal()` means you can't use the list and the map together. Make it a non-modal side drawer: compact rows (currently ~90px tall — 25 nodes ≈ 10 screens of scrolling), grouped by type/region, hover-row → highlight node on map, and a search field. |
There was a problem hiding this comment.
For the audited 820px desktop canvas, this is already a non-modal drawer: openNodeList calls showModal() only when isTouchMap() is true and otherwise merely sets the dialog's open attribute. Describing the List as universally blocking and scheduling a conversion to a non-modal drawer therefore turns behavior already present on desktop into implementation work; limit this finding to the touch layout if making the mobile sheet non-modal is actually desired.
Useful? React with 👍 / 👎.
|
|
||
| Measured live at 820px canvas width: **35 node pairs within 18px of each other; four pairs at exactly 0px.** | ||
|
|
||
| - `Greenpill NYC (chapter) ↔ Matty Compost (member): 0px` — **Matt's own node is invisible under the NYC chapter dot.** This is C6: the people layer isn't sparse, it's occluded. |
There was a problem hiding this comment.
At exact chapter/person coordinates, the person is not painted under the chapter: HomeMap.astro places the dynamic members group after the chapters group, so normal SVG paint order renders the member on top, as this document itself acknowledges on line 42. The claim that Matt's node is invisible under the NYC dot—and the resulting conclusion that the people layer only appears sparse because chapters hide people—therefore gives the clustering plan an incorrect diagnosis.
Useful? React with 👍 / 👎.
| ### 2.3 Information layout & overlays | ||
|
|
||
| - **P1 · Hover card is oversized and occluding:** hovering kitblake produced a ~320×190px card covering ~40% of the map (name, bioregion, role, 4 theme chips). Browsing = repeatedly blinding yourself. Make hover a **compact tooltip** (name + `TYPE · PLACE`), and move the rich content (bioregion, themes, connections) to the selected state. | ||
| - **P0 · Hover card gets stuck:** the kitblake card persisted through clicks, filter changes, opening the Themes panel, opening the List modal, and page scroll — stale UI layered over everything (canvas held `is-focused`). Clear focus state on pointer-leave, scroll, filter change, and any overlay opening. Same for **selection threads**: the teal/orange strands from a previous selection survived closing the inspector and even viewport changes. |
There was a problem hiding this comment.
The current inspector close path already clears these strands: closeSelected() nulls the selected target and calls clearFocusState(), which invokes clearSelectionStrands(). If the strands persisted in the older production build noted at the start of the audit, this should be recorded as production-only or already fixed rather than retained as a P0 task against local main, where it would prompt a redundant lifecycle rewrite.
Useful? React with 👍 / 👎.
|
|
||
| - **P1 · Hover card is oversized and occluding:** hovering kitblake produced a ~320×190px card covering ~40% of the map (name, bioregion, role, 4 theme chips). Browsing = repeatedly blinding yourself. Make hover a **compact tooltip** (name + `TYPE · PLACE`), and move the rich content (bioregion, themes, connections) to the selected state. | ||
| - **P0 · Hover card gets stuck:** the kitblake card persisted through clicks, filter changes, opening the Themes panel, opening the List modal, and page scroll — stale UI layered over everything (canvas held `is-focused`). Clear focus state on pointer-leave, scroll, filter change, and any overlay opening. Same for **selection threads**: the teal/orange strands from a previous selection survived closing the inspector and even viewport changes. | ||
| - **P1 · Selected inspector is a center modal:** selecting a node opens a large card over the middle of the map — you lose the geography (where is this node?), it covers the legend, and its connections list clips mid-row with chips overflowing the card edge. Dock it to a side rail (desktop) so the selected node + its highlighted threads stay visible; fix the internal overflow. (Mobile's bottom-sheet pattern is the right idea — desktop should get the equivalent side dock.) Also: the closed inspector leaves 0×0 but still-`aria-pressed` connection buttons in the DOM — remove or `inert` them. |
There was a problem hiding this comment.
Closing the inspector sets its hidden attribute, and the component explicitly styles .gp-home-map-selected[hidden] with display: none; the connection buttons consequently leave both the focus order and accessibility tree even if their DOM nodes retain aria-pressed. The proposed removal or inert work addresses a ghost-control problem that the audited implementation does not have.
Useful? React with 👍 / 👎.
| **Why, in code:** | ||
| - The overlap-stack mechanism only groups **people**, and only at **bit-identical coordinates** (`groupPins` keys on `toFixed(2)` — `HomeMap.astro:2782, 2796-2803`). Two members 0.5px apart never cluster. Chapters are excluded entirely (different SVG group, `visiblePeoplePins` queries members only, `2784-2793`). | ||
| - Chapters are also excluded from the density-shrink pass (`1015-1022`) — they never shrink, never move, and members paint **after** chapters in DOM order (`218-250`), so members visually cover chapters while chapters' larger hit circles lose the pointer contest: clicking the center of the NA cluster selected **Metafortune77**, not any of the four chapters. Chapters are effectively unclickable in dense regions. | ||
| - **P0 · Fix direction — cluster across types, by radius:** grid/radius grouping (~2.5 SVG units) over *all* node types; render a cluster pin (chapter-anchored, count badge, blended theme ring) that fans out satellites on click/tap (the satellite mechanics already exist for exact-overlap people stacks, `2767-2949` — generalize them). When ≥2 nodes fall within the 44px pointer radius, open the fan or a small chooser instead of nearest-center-wins (`nodeAtPointer`, `1621-1633`). |
There was a problem hiding this comment.
A fixed radius of roughly 2.5 SVG units does not implement the stated visual or pointer-radius criterion: with the 200-unit viewBox it is only about 4.7px on the 375px canvas and 10.3px on the measured 820px canvas, both below the audit's 18px collision set and far below the 44px client-space pointer radius. It also changes effective size whenever the map zooms. If this P0 plan is implemented as written, many of the reported overlaps will remain separate; derive the grouping threshold from client pixels through the current viewBox scale and recompute it on zoom/resize.
Useful? React with 👍 / 👎.
|
|
||
| **P1 · Edge cases found live:** | ||
| - **Clipped node:** member "Amio" renders half outside the canvas edge (far-west longitude). Clamp x into the drawable inset or wrap threads/nodes; never clip a pin. | ||
| - **Duplicate identity:** "kitblake" appears at two locations (Amsterdam ~Germany, and at Emmanuel Jacobson's exact coords in West Africa). Dedup by identity at injection (`3302-3308` exists but missed this) and flag same-name/same-source nodes. |
There was a problem hiding this comment.
The browser has no safe identity key with which to perform the proposed deduplication: toMember() keys approved nodes by public submission/source ID, while names are non-unique and the owner email is deliberately absent from the public payload. Treating equal display names as one identity could silently remove legitimate people, and exposing a private identifier to fix that would violate the privacy boundary. Resolve confirmed duplicate submissions during moderation or define an appropriate shared public contract rather than deduplicating inferred identities at injection.
AGENTS.md reference: AGENTS.md:L139-L143
Useful? React with 👍 / 👎.
1 participant
Closes PRD-756.
Every routine spec and agent-facing doc told writers to apply
agent:routine/agent:codex/agent:claude. Noagentlabel group exists in Linear. The live group isai, with exactly those three children. Becausesave_issuerejects the entire label array when any one entry fails to resolve, an agent following the docs literally got a hard write failure and filed nothing. That is howhealth-watchhit this while filing PRD-755.Verified against live Linear before changing anything: the
aigroup holdsroutine(90260e40…),codex(50724377…),claude(33e73a3a…), and there is noagentgroup.Two traps this rename had to avoid
package:agentis a real, unrelated label (the agent runtime package) and is untouched. This was not a find-and-replace of the word "agent"; the substitutions were scoped to the four label tokens.agent:copilotappeared in the operating model as an example and has noai:equivalent at all. The live group is only routine/codex/claude, so it was dropped rather than translated.The second finding, also from PRD-756
The
group:childnotation these docs use throughout is display shorthand, not accepted API input.save_issueresolves labels by bare child name or ID, so["protocol:green-goods", "activity:qa"]fails where["green-goods", "qa"]succeeds. PRD-756's own label array reads[routine, qa, docs, green-goods], which is the proof. This is now stated at the canonical definition and at both routine-facing indexes, since that is where writers actually look.One further correction found while reading
The guild routines README said to "optionally add
ai:claudeorai:codex" on top ofai:routine. Linear enforces one child per grouped family, so that write would be rejected for exactly the same reason as the original bug. It now says they replace rather than accompany.Not in this PR
green-goods/.plans/active/commitment-pooling/**also carriesagent:*references. Left alone deliberately: the audit reports andstatus.jsonevent log are dated historical records that should not be rewritten, and the lane data is live input toplan-hub.mjs linear-syncfor a feature another agent is actively working. It needs its own scoped pass, with theplan-hub.test.mjsfixtures moving in lockstep.🤖 Generated with Claude Code