Skip to content

docs(plan): Phase 17 — Graph as entity-scoped tab + Source migration (full GSD planning bundle)#230

Open
damienriehl wants to merge 10 commits into
devfrom
chore/plan-phase-17
Open

docs(plan): Phase 17 — Graph as entity-scoped tab + Source migration (full GSD planning bundle)#230
damienriehl wants to merge 10 commits into
devfrom
chore/plan-phase-17

Conversation

@damienriehl
Copy link
Copy Markdown
Contributor

@damienriehl damienriehl commented May 2, 2026

Sets up #229. Zero code changes.planning/ artifacts only. Direct-commit to dev was blocked by branch protection ("4 of 4 required status checks are expected"); this PR is the path the repo policy enforces.

What's in this PR

The full Get-Shit-Done planning chain for Phase 17 — design contract, sketches, spec, context, UI spec, validation strategy, and three execution plans. No implementation code yet — that lands in a separate feature branch after this merges, on top of PR #88 (entity graph) once it's in dev.

Design (4 throwaway HTML mockups + decision contract)

File Purpose
.planning/notes/graph-as-entity-pane.md 11-decision design contract
.planning/sketches/001-tab-strip-language/ Winner D — icon + sentence-case label, ABOVE entity header
.planning/sketches/002-standard-view-layout/ Winner D — Compact density + 50/50 tabs, bottom borders aligned
.planning/sketches/003-developer-view-and-modal/ Applied + Source-as-third-tab restructure
.planning/sketches/004-open-full-source/ Winner C — Modal ⇄ Maximize round-trip with Restore

Open any sketch's index.html directly in a browser; corner toolbar toggles dark mode.

Phase artifacts (.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/)

File Step Purpose
17-SPEC.md spec-phase 11 falsifiable requirements + 23 pass/fail acceptance criteria; ambiguity 0.19
17-CONTEXT.md discuss-phase Implementation decisions extracted from Socratic discussion
17-DISCUSSION-LOG.md discuss-phase Audit trail of questions/answers
17-PATTERNS.md plan-phase Pattern map — closest analogs in the codebase for new files
17-RESEARCH.md plan-phase Research notes consumed by planner
17-UI-SPEC.md ui-phase Tab strip token spec, modal round-trip contract, edge palette
17-VALIDATION.md (separate) Validation strategy across the three plans
17-01-PLAN.md plan-phase Wave 1 — Backend (ontokit-api): extend entity-graph endpoint for property + individual focus IRIs
17-02-PLAN.md plan-phase Wave 2 — Foundation (web): selectionStore extension, PaneTabStrip + SourceTabBody components, hooks (useFullSourceOverlay, useEffectiveTab), utility (extractEntitySnippet)
17-03-PLAN.md plan-phase Wave 3 — Integration (web): wire up StandardEditorLayout / DeveloperEditorLayout / detail panels; AI smoke test + UAT checklist

.planning/ROADMAP.md updated with the Phase 17 entry. .planning/STATE.md carries session bookkeeping for each step.

Cross-repo scope

Wave 1 lives in CatholicOS/ontokit-api — the backend entity-graph endpoint (currently class-only) needs to handle property and individual focus IRIs with full domain/range/parent/see-also (properties) and rdf:type/object-property/sameAs (individuals). That work depends on this PR for the contract and spec; the api PR will be filed separately once this merges.

Waves 2-3 live in ontokit-web, on a new feature branch cut from dev after PR #88 (entity graph server-side BFS) merges. Hard dependency.

Why merge planning before implementation

  1. Contract review window. Catching a misaligned decision now — particularly Decisions 9 (Source migrating into the right pane) and 10 (Modal ⇄ Maximize round-trip with "Restore" terminology) — costs minutes; catching it during implementation review costs days.
  2. Cross-repo coordination. Wave 1 (api) and Wave 2 (web) are scoped from the same SPEC. Both repos read from the merged contract.
  3. Sketches as build reference. The four sketches stay live on dev so anyone implementing or reviewing can open .planning/sketches/{NNN}/index.html and see the target design.

Tracker + issue

What's NOT in this PR

  • No .tsx / .py source changes
  • No test changes (other than the validation strategy doc)
  • No package.json / lockfile changes
  • No CI changes

Implementation lands separately:

🤖 Generated with Claude Code

…to right pane

Adds the design contract and four validation sketches for Phase 17 (v0.5.0):
move the entity graph and the source view from their current homes (peer-tab
in Developer + hidden icon in Standard for graph; left-pane Tree|Source mode
strip + entity-actions </> Source link for source) into a unified right-pane
tab strip — Detail | Graph in Standard, Detail | Graph | Source in Developer.

Design contract: .planning/notes/graph-as-entity-pane.md
  · 11 decisions covering layout, click semantics, tab persistence,
    Source-as-Developer-tab, modal⇄Maximize round-trip with Restore
    terminology, and tree→source auto-jump behavior spec.

Sketches: .planning/sketches/{001..004}/index.html
  · 001 tab-strip-language → winner D (icon + label, ABOVE entity header)
  · 002 standard-view-layout → winner D (Compact density + 50/50 tabs)
  · 003 developer-view-and-modal → applied + Source-as-third-tab restructure
  · 004 open-full-source → winner C (Modal ⇄ Maximize round-trip with
    "Restore" matching Windows OS convention)

Roadmap: appended Phase 17 entry.

Tracker: ontokit-web#229 (under v0.5.0 milestone #214).

No code changes — design and planning artifacts only. Implementation lands
on its own feature branch via /gsd-spec-phase → /gsd-discuss-phase →
/gsd-plan-phase → /gsd-execute-phase, gated on PR #88 (entity graph) merging
to dev first.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 2, 2026

📝 Walkthrough

Walkthrough

Adds Phase 17 planning, specs, and interactive sketches to relocate the entity graph into a right-pane entity-scoped "Detail | Graph" tab strip (Developer mode adds a Source tab), defines modal ⇄ maximize round-trip for full source, updates roadmap/STATE, and records API/backend contract and test plans.

Changes

Phase 17 Planning & Sketches

Layer / File(s) Summary
Roadmap Integration
.planning/ROADMAP.md
Adds Phase 17 ("Graph as Entity-Scoped Tab in Detail Pane") with milestone v0.5.0 and appends Phase 17 to Execution Order.
Phase Context & Spec
.planning/phases/17-.../17-CONTEXT.md, .planning/phases/17-.../17-SPEC.md, .planning/phases/17-.../17-UI-SPEC.md
Defines Phase 17 scope, locked requirements R1–R11, UI contracts (PaneTabStrip, SourceTabBody, EntityModal rename), backend /entity-graph contract (focus_type/focus_iri), and acceptance criteria.
Execution Plans
.planning/phases/17-.../17-01-PLAN.md, 17-02-PLAN.md, 17-03-PLAN.md, 17-VALIDATION.md
Details backend API tasks, scaffolding wave (hooks, store, components, tests), integration wave (layout wiring, removals, UAT/smoke tests), test matrices, and verification sign-off procedures.
Design Notes & Patterns
.planning/notes/graph-as-entity-pane.md, .planning/phases/17-.../17-PATTERNS.md, 17-DISCUSSION-LOG.md, 17-RESEARCH.md
Captures UI decisions (tab placement above header, single/double-click semantics), snippet extraction strategy, modal takeover mechanics, selection-store activePaneTab semantics, edge-kind palette, and implementation constraints/landmines.
Sketch Manifest & Theme
.planning/sketches/MANIFEST.md, .planning/sketches/themes/default.css
Adds sketch manifest and a sketch theme CSS with light/dark tokens, typography, spacing, and fixed sketch UI elements.
Sketches — Tab Strip & Layout
.planning/sketches/001-tab-strip-language/*, .planning/sketches/002-standard-view-layout/*
Interactive HTML/CSS sketches demonstrating tab-strip language and Standard view two-pane layout (Variant D winners, tab density, 50/50 full tabstrip behavior).
Sketches — Developer View & Modal
.planning/sketches/003-developer-view-and-modal/*
Sketch showing Developer mode 3-tab strip (Detail
Sketches — Open Full Source Round-Trip
.planning/sketches/004-open-full-source/*
Sketch implementing modal ↔ maximize ↔ restore round-trip (Variant C winner), CAME_FROM_MODAL state, line highlighting, auto-scroll on selection changes, and overlay dismissal rules.
State Update
.planning/STATE.md
Updates milestone progress to reflect Phase 17 plans approved, updates last_updated, Last session, and resume file to Phase 17 plan.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

Suggested labels

UX

Poem

🐰 A sketch, a plan, a well-marked road,
Where graphs find homes, no longer stowed,
Detail beside, with Source to see,
Tabs hop in tune—what design glee! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: addition of Phase 17 planning documentation for Graph-as-entity-scoped tab and Source migration, accurately reflecting the extensive planning bundle in .planning/.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/plan-phase-17

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (1)
.planning/sketches/004-open-full-source/README.md (1)

56-63: ⚡ Quick win

Add modal accessibility requirements to implementation watch-outs.

Please include explicit focus-management/ARIA expectations (focus trap, initial focus target, return focus on close, role="dialog" + aria-modal) for both modal and takeover transitions so the round-trip doesn’t regress keyboard/screen-reader flows.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/sketches/004-open-full-source/README.md around lines 56 - 63, Add
explicit accessibility/focus-management rules to the modal/takeover
implementation: in the reused EntityGraphModal component (and the takeover
variant created when Maximize is invoked) ensure role="dialog" and aria-modal
are set, implement a focus trap while open, set a clear initial focus target
inside the modal body (e.g., first interactive element passed via the body slot
or a dedicated focusable container), and restore focus to the element that
opened the modal when it closes or when Restore toggles back; wire this into the
source-overlay state (cameFromModal boolean) so toggling between Maximize and
Restore preserves focus and scroll position, and ensure the global Esc handler
still dismisses the entire source overlay (not just shrink to modal) while
Restore only changes size without moving return focus unexpectedly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.planning/ROADMAP.md:
- Line 208: Update the Phase 17 dependency statement to reflect the actual PR
gate: replace or modify the line "Depends on: none" in .planning/ROADMAP.md so
it references PR `#88` (entity graph) or otherwise marks the entity graph as a
prerequisite; ensure the text explicitly matches the PR gate (e.g., "Depends on:
PR `#88` (entity graph)") so scheduling and parallelization tools/readers will not
treat Phase 17 as independent.
- Line 211: The Phase 17 success criteria in ROADMAP.md conflicts with the
design contract: update the text so Developer view does not reduce the
right-pane tabs to "Tree | Source" but instead adopts the canonical "Detail |
Graph | Source" on the right-side detail pane (and remove any reference to a
left-pane "Tree | Source" tab strip for Developer layout); ensure the doc
explicitly states the Developer layout moves "Source" into the right-pane tabs
and that left-pane tab strip is removed so the behavior matches the design note
and avoids implementation drift.

In @.planning/sketches/002-standard-view-layout/README.md:
- Line 27: The spec for the Compact tab style is ambiguous about vertical
padding (it lists 8px in the Compact definition but later expects ~11px); pick
one consistent value or explicitly scope both by updating the "Compact"
definition and any later mentions so they match (e.g., change the Compact bullet
to "Compact — text-xs tab labels, 24px entity pip, 11px tab vertical padding" or
add a note like "8px applies to collapsed tabs, 11px applies to expanded tabs"),
and ensure every reference to Compact or tab padding in this file (including the
later ~11px mention) uses the same chosen value or the scoped explanation.

In @.planning/sketches/003-developer-view-and-modal/README.md:
- Around line 54-55: Update the README reference so implementers are directed to
the correct module: replace the mention of turtleClassUpdater.ts with
turtleBlockParser.ts and note that the existing helper findBlock(...) in
turtleBlockParser.ts should be reused when implementing
getEntitySourceSnippet(iri, ttlSource); also mention that findBlock is the
function to call to locate the Turtle block for an IRI and that the "Open full
source" affordance should reuse the existing full-file Source view logic and
scroll to the found block.

In @.planning/sketches/004-open-full-source/index.html:
- Around line 503-506: Rename all uses of the rejected "minimize" terminology to
"restore": change the element ID takeover-minimize-btn to takeover-restore-btn,
the CSS class .minimize-btn to .restore-btn, and the function minimizeToModal()
to restoreToModal(); update any HTML attributes (id, class, onclick/title text
references), corresponding JS references (function definition, calls, event
listeners) and any CSS selectors that target .minimize-btn so they target
.restore-btn instead to keep names consistent.

In @.planning/sketches/MANIFEST.md:
- Line 5: The MANIFEST entry for Phase 17 incorrectly says "eight" decisions;
update the sentence in .planning/sketches/MANIFEST.md that reads 'Read
`.planning/notes/graph-as-entity-pane.md` for the eight design decisions' to say
"eleven" to match the actual document; verify the referenced note file
`.planning/notes/graph-as-entity-pane.md` (whose heading reads "The design
contract (11 decisions)") is the source of truth and keep the rest of the
sentence unchanged.

---

Nitpick comments:
In @.planning/sketches/004-open-full-source/README.md:
- Around line 56-63: Add explicit accessibility/focus-management rules to the
modal/takeover implementation: in the reused EntityGraphModal component (and the
takeover variant created when Maximize is invoked) ensure role="dialog" and
aria-modal are set, implement a focus trap while open, set a clear initial focus
target inside the modal body (e.g., first interactive element passed via the
body slot or a dedicated focusable container), and restore focus to the element
that opened the modal when it closes or when Restore toggles back; wire this
into the source-overlay state (cameFromModal boolean) so toggling between
Maximize and Restore preserves focus and scroll position, and ensure the global
Esc handler still dismisses the entire source overlay (not just shrink to modal)
while Restore only changes size without moving return focus unexpectedly.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 71a4d8fd-b2f8-4046-9d3c-13c523b9487a

📥 Commits

Reviewing files that changed from the base of the PR and between 795c973 and f3a17a6.

📒 Files selected for processing (12)
  • .planning/ROADMAP.md
  • .planning/notes/graph-as-entity-pane.md
  • .planning/sketches/001-tab-strip-language/README.md
  • .planning/sketches/001-tab-strip-language/index.html
  • .planning/sketches/002-standard-view-layout/README.md
  • .planning/sketches/002-standard-view-layout/index.html
  • .planning/sketches/003-developer-view-and-modal/README.md
  • .planning/sketches/003-developer-view-and-modal/index.html
  • .planning/sketches/004-open-full-source/README.md
  • .planning/sketches/004-open-full-source/index.html
  • .planning/sketches/MANIFEST.md
  • .planning/sketches/themes/default.css

Comment thread .planning/ROADMAP.md
### Phase 17: Graph as Entity-Scoped Tab in Detail Pane
**Milestone**: v0.5.0
**Goal**: Move the entity graph from a peer-tab in Developer view (and a hidden icon button in Standard view) into a `Detail | Graph` tab strip at the top of the right-side detail pane, applied uniformly across both views and across all three entity types (class, property, individual). Property graphs render real domain/range/parent-property edges (an explicit improvement over WebProtege's empty-island treatment).
**Depends on**: none (touches Standard and Developer layouts independently of other Phase 17+ work)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align Phase 17 dependency with the stated PR gate.

Depends on: none conflicts with this PR’s stated implementation gate on PR #88 (entity graph). This can cause incorrect scheduling/parallelization assumptions in planning.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/ROADMAP.md at line 208, Update the Phase 17 dependency statement
to reflect the actual PR gate: replace or modify the line "Depends on: none" in
.planning/ROADMAP.md so it references PR `#88` (entity graph) or otherwise marks
the entity graph as a prerequisite; ensure the text explicitly matches the PR
gate (e.g., "Depends on: PR `#88` (entity graph)") so scheduling and
parallelization tools/readers will not treat Phase 17 as independent.

Comment thread .planning/ROADMAP.md

## Variants

- **A: Compact** — text-xs tab labels, 24px entity pip, 8px tab padding. Tabs collapsed to the left, content-sized.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Unify tab vertical padding spec (8px vs ~11px).

The file defines Compact as 8px but later asks to keep ~11px. Pick one value (or explicitly scope when each applies) so implementation and visual QA don’t diverge.

Also applies to: 45-45

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/sketches/002-standard-view-layout/README.md at line 27, The spec
for the Compact tab style is ambiguous about vertical padding (it lists 8px in
the Compact definition but later expects ~11px); pick one consistent value or
explicitly scope both by updating the "Compact" definition and any later
mentions so they match (e.g., change the Compact bullet to "Compact — text-xs
tab labels, 24px entity pip, 11px tab vertical padding" or add a note like "8px
applies to collapsed tabs, 11px applies to expanded tabs"), and ensure every
reference to Compact or tab padding in this file (including the later ~11px
mention) uses the same chosen value or the scoped explanation.

Comment on lines +54 to +55
- **Entity-scoped source rendering.** New: needs a function `getEntitySourceSnippet(iri, ttlSource)` that finds the Turtle block for an IRI. Already exists in `lib/ontology/turtleClassUpdater.ts` (`findBlock`) — reuse it for the Source tab.
- **"Open full source" affordance.** Clicking it should open the existing full-file Source view in a modal or a new pane, scrolled to the entity's line. This preserves the "I need the whole file" workflow without making it the default.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix findBlock source-file reference.

This points implementers to lib/ontology/turtleClassUpdater.ts, but current usage evidence is in lib/ontology/turtleBlockParser.ts (where findBlock(...) is used). Please correct the reference to prevent wrong dependency assumptions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/sketches/003-developer-view-and-modal/README.md around lines 54 -
55, Update the README reference so implementers are directed to the correct
module: replace the mention of turtleClassUpdater.ts with turtleBlockParser.ts
and note that the existing helper findBlock(...) in turtleBlockParser.ts should
be reused when implementing getEntitySourceSnippet(iri, ttlSource); also mention
that findBlock is the function to call to locate the Turtle block for an IRI and
that the "Open full source" affordance should reuse the existing full-file
Source view logic and scroll to the found block.

Comment on lines +503 to +506
<button id="takeover-minimize-btn" class="minimize-btn" onclick="minimizeToModal()" title="Restore to modal view" style="display:none;">
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" style="vertical-align:-2px;"><polyline points="4 14 10 14 10 20"/><polyline points="20 10 14 10 14 4"/><line x1="14" y1="10" x2="21" y2="3"/><line x1="3" y1="21" x2="10" y2="14"/></svg>
Restore
</button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

CSS class, element ID, and function name use the rejected "minimize" terminology.

Decision 10 explicitly argues for "Restore" and against "Minimize" (OS muscle-memory conflict). The button label and title are correct (Restore / "Restore to modal view"), but the CSS class (.minimize-btn), element ID (takeover-minimize-btn), and minimizeToModal() function all use the rejected term. Implementors following this sketch will likely carry this naming into production.

✏️ Proposed alignment
-          <button id="takeover-minimize-btn" class="minimize-btn" onclick="minimizeToModal()" title="Restore to modal view" style="display:none;">
+          <button id="takeover-restore-btn" class="restore-btn" onclick="restoreToModal()" title="Restore to modal view" style="display:none;">

And in the JS:

-    const minBtn = document.getElementById('takeover-minimize-btn');
+    const minBtn = document.getElementById('takeover-restore-btn');
-    function minimizeToModal() {
+    function restoreToModal() {

Also update the CSS selector:

-    .modal-actions .maximize-btn,
-    .modal-actions .minimize-btn {
+    .modal-actions .maximize-btn,
+    .modal-actions .restore-btn {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/sketches/004-open-full-source/index.html around lines 503 - 506,
Rename all uses of the rejected "minimize" terminology to "restore": change the
element ID takeover-minimize-btn to takeover-restore-btn, the CSS class
.minimize-btn to .restore-btn, and the function minimizeToModal() to
restoreToModal(); update any HTML attributes (id, class, onclick/title text
references), corresponding JS references (function definition, calls, event
listeners) and any CSS selectors that target .minimize-btn so they target
.restore-btn instead to keep names consistent.


## Design Direction

Phase 17 — move the entity graph from a peer-tab in Developer view (and a small icon button in Standard view) into a `Detail | Graph` tab strip at the top of the right-side detail pane. Visual language matches OntoKit's existing app: sky-blue primary (`--color-primary-*`), slate neutrals, OWL-entity accent colors (class=green, property=blue, individual=pink), light + dark via CSS data-attribute. Folio-Enrich tab strip is the closest reference; WebProtege's docked entity graph is the loose mental model. Read `.planning/notes/graph-as-entity-pane.md` for the eight design decisions that constrain these sketches.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Stale decision count — "eight" should be "eleven".

graph-as-entity-pane.md documents 11 decisions (its own heading reads "The design contract (11 decisions)"). Decisions 9–11 were added during the sketch session, but the MANIFEST reference still says "eight."

✏️ Proposed fix
-Phase 17 — move the entity graph … Read `.planning/notes/graph-as-entity-pane.md` for the eight design decisions that constrain these sketches.
+Phase 17 — move the entity graph … Read `.planning/notes/graph-as-entity-pane.md` for the eleven design decisions that constrain these sketches.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/sketches/MANIFEST.md at line 5, The MANIFEST entry for Phase 17
incorrectly says "eight" decisions; update the sentence in
.planning/sketches/MANIFEST.md that reads 'Read
`.planning/notes/graph-as-entity-pane.md` for the eight design decisions' to say
"eleven" to match the actual document; verify the referenced note file
`.planning/notes/graph-as-entity-pane.md` (whose heading reads "The design
contract (11 decisions)") is the source of truth and keep the rest of the
sentence unchanged.

damienriehl and others added 9 commits May 2, 2026 12:10
…Pane — 11 requirements

Locks WHAT/WHY for Phase 17 (v0.5.0). 11 falsifiable requirements derived
from the design contract (.planning/notes/graph-as-entity-pane.md) + 2
rounds of Socratic refinement that pinned the cross-repo scope and
verification floor.

Key locked decisions surfaced in spec round 1-2:
  · Phase 17 spans both ontokit-web and ontokit-api (web restructure +
    api endpoint extension for property/individual focus IRIs)
  · Hard dependency on PR #88 (entity graph) merging to dev before
    Phase 17 implementation branch is cut
  · Verification floor: Vitest unit tests + AI-driven Chrome DevTools
    MCP smoke pass walking the success criteria in real browser before
    human UAT (no Playwright e2e in this phase)
  · Property graphs render full content per Decision 5: domain + range
    + parent properties + see-also (improvement over WebProtege's
    empty-island treatment)

Ambiguity: 0.19 (gate ≤ 0.20). All 4 dimensions cleared their minimums.

Tracker: ontokit-web#229 (under v0.5.0 milestone #214).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…coped-tab-in-detail-pane/17-CONTEXT.md .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-DISCUSSION-LOG.md
…e palette

Locks visual + interaction contract for Phase 17. Concrete Tailwind
classes for the right-pane PaneTabStrip (mirrors EntityTabBar density
so bottom borders align), EntityModal rename + Maximize/Restore
semantics, Source tab toolbar layout, edge palette extension for
domain/range/subPropertyOf/rdfType/sameAs/objectProperty, empty-state
copy for annotation properties, and full a11y/copy contracts.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…entity-scoped-tab-in-detail-pane/17-VALIDATION.md
…l-pane

Three-plan structure honoring SPEC R1-R11 + CONTEXT D-01..D-16:

- 17-01-PLAN.md (cross-repo, autonomous: false, wave 1):
  Extend ontokit-api /entity-graph endpoint for property + individual focus,
  add 6 new edge_kind values (domain/range/subPropertyOf/rdfType/sameAs/
  objectProperty), preserve /classes/graph as deprecating shim, add 14
  pytest cases covering R11 acceptance literal. Sequential merge gate
  per D-08: api -> dev BEFORE web cuts its branch.

- 17-02-PLAN.md (autonomous: true, wave 2, depends_on=17-01):
  Web primitives layer. PaneTabStrip + SourceTabBody + FullSourceOverlay
  + useFullSourceOverlay hook + selectionStore activePaneTab field +
  useEffectiveTab derived fallback (D-15) + EntityGraphModal -> EntityModal
  rename + OntologyEdge palette extension to 10 edge_kind values. CSS
  takeover rule (data-overlay-takeover) added to globals.css. 8 tasks +
  ~60 unit tests, no layout integration yet.

- 17-03-PLAN.md (autonomous: false due to UAT checkpoint, wave 3,
  depends_on=17-02):
  Layout integration. Wire PaneTabStrip into Standard (Detail|Graph) and
  Developer (Detail|Graph|Source). Remove old Graph icon button (R3),
  Source link (R4), and Developer Tree|Source|Graph mode strip (R2).
  Wire useGraphData to pass focusType through. AI-driven Chrome DevTools
  MCP smoke test report covering R1, R5, R6, R7, R8, R9 across
  Standard+Developer x light+dark = 24-scenario matrix. Human UAT
  checklist mapped 1:1 to R1-R11.

All three plans validate clean (gsd-sdk verify.plan-structure: valid=true).
ROADMAP.md updated: Requirements: R1..R11; Plans: 3 plans.

Phase artifacts cited: 17-SPEC.md (locked R1-R11), 17-CONTEXT.md (16
locked decisions), 17-RESEARCH.md (file paths, line numbers, pitfalls),
17-PATTERNS.md (analog files + code excerpts), 17-VALIDATION.md
(Nyquist requirement-to-test mapping), 17-UI-SPEC.md (Tailwind tokens,
ARIA roles, copy contract, locked edge palette).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- B1: 17-02 frontmatter requirements expanded to [R1, R6, R7, R8, R9, R10, R11]
  (Task 6 PaneTabStrip is load-bearing for R1; Task 7 FullSourceOverlay
  auto-scroll implements R10).
- B2: 17-02 Task 7 — added Test 15 (Monaco instance ref persists across
  Maximize⇄Restore round-trip) and Test 16 (editor-tree-pane and
  editor-detail-pane testids remain in DOM during maximize). Concrete
  greppable test-name strings added to acceptance.
- B3: 17-02 Task 8 — reconciled SPEC R11 vs UI-SPEC empty-state copy
  collision. SPEC literal now ships as aria-label on container; UI-SPEC
  heading + body remain visible. Tests grep for all three strings.
- B4: 17-02 Task 7 — removed TODO escape hatch for R10 auto-scroll.
  Now mandates TurtleEditor refactor to forwardRef + useImperativeHandle
  exposing revealLineInCenter (mirrors existing OntologySourceEditor
  pattern). Added Test 17 (focusIri change calls revealLineInCenter).
- B5: 17-03 Task 1 — added positive-absence assertions
  (queryByText / queryByLabelText / queryByRole) for the Source link in
  PropertyDetailPanel.test.tsx, IndividualDetailPanel.test.tsx, and
  ClassDetailPanel.test.tsx. Replaces the vacuous grep-of-absent-string.

All three plans pass frontmatter + structure validation.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@damienriehl damienriehl changed the title docs(plan): Phase 17 — Graph as entity-scoped tab + Source migration (design contract + sketches) docs(plan): Phase 17 — Graph as entity-scoped tab + Source migration (full GSD planning bundle) May 2, 2026
@damienriehl damienriehl requested a review from JohnRDOrazio May 2, 2026 18:56
@damienriehl
Copy link
Copy Markdown
Contributor Author

@JohnRDOrazio review request — this is the full GSD planning bundle for Phase 17 (entity graph + Source moved into a unified right-pane tab strip in both Standard and Developer views). Zero code, all design + spec + plans. The four sketches under .planning/sketches/ are interactive HTML — open .planning/sketches/004-open-full-source/index.html to feel the Modal ⇄ Maximize round-trip, which is the most novel piece. The 11-decision design contract in .planning/notes/graph-as-entity-pane.md is the central artifact; everything else (SPEC, CONTEXT, UI-SPEC, plans) flows from it. Catching a misaligned call here costs minutes; catching it during implementation costs days. Tracker: #229.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-01-PLAN.md:
- Around line 7-12: The plan's metadata key files_modified is missing the
required test artifact tests/unit/test_graph_schema.py referenced later (lines
375–377); update the files_modified list to include
tests/unit/test_graph_schema.py alongside the existing entries
(ontokit/api/routes/projects.py, ontokit/services/ontology.py,
ontokit/schemas/graph.py, tests/unit/test_entity_graph.py) so the execution
tooling will include and run the schema test.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-02-PLAN.md:
- Line 263: The markdown uses indented code blocks (e.g., the example line
adding the PaneTab type export `export type PaneTab = "detail" | "graph" |
"source";`) which triggers MD046; replace each indented snippet with a fenced
code block using triple backticks and an appropriate language tag (```ts for
TypeScript examples) so the snippet is fenced and syntax-highlighted; apply this
change to the mentioned occurrences (lines cited in the review) to ensure
consistent fenced blocks throughout the document.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-03-PLAN.md:
- Around line 808-814: The table cell in the T-17-13 row currently contains the
unescaped union literal class"|"property"|"individual which is breaking markdown
table parsing and triggering MD056/MD055; edit the T-17-13 cell (the
`useGraphData focusType param threading` row) and either escape the pipe
characters as class"\|"\|property"\|"\|individual or wrap the entire union in
inline code/backticks (e.g., `class|property|individual`) so the pipes are not
treated as column separators.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-CONTEXT.md:
- Around line 69-72: Normalize the activePaneTab contract by removing null as an
allowed value and making activePaneTab strictly 'detail' | 'graph' | 'source' in
useSelectionStore and all phase docs; ensure useSelectionStore initializes to
'detail' and that any reset logic (referenced by D-14/D-15) explicitly sets
activePaneTab = 'detail', and update useEffectiveTab(editorMode) wording to
treat activePaneTab as always present (only deriving a fallback from 'source'
when editorMode === 'standard') so downstream plans/tests assume a non-null
default.
- Around line 51-52: The comment flags conflicting guidance: don't re-implement
type-agnostic block-finding in lib/ontology/turtleClassUpdater.ts; instead adapt
the existing turtleUtils.findBlock wrapper to support both property and
individual-subject patterns and call that from turtleClassUpdater.ts; update
turtleClassUpdater.ts to use turtleUtils.findBlock (not a new findBlock) and
ensure it consumes the Web Worker IRI index from lib/editor/indexWorker.ts for
start line numbers; also adjust any references at lines ~102-103 and ~125-126 to
match the unified wrapper approach so all callers share the same type-agnostic
contract.

In
@.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-PATTERNS.md:
- Around line 708-712: The fenced code block that currently starts with the
lines containing the Tailwind utility classes (e.g., "flex border-b
border-slate-200 dark:border-slate-700" and "flex-1 px-3 py-2 text-xs
font-medium transition-colors") is missing a language identifier; update the
opening ``` fence to include a language such as css or html (e.g., ```css) to
satisfy MD040 and enable proper syntax highlighting for the utility-class
snippet.

In
@.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-RESEARCH.md:
- Line 42: The MD040/MD058 warnings are caused by missing fenced-code language
identifiers and tables lacking surrounding blank lines; add explicit language
tags (e.g., ```md or ```text) to every fenced code block and ensure there is a
blank line immediately before and after each markdown table (for example the
table starting with "| Library | Version | Purpose | Why Standard |"); apply the
same fixes to the other table occurrences noted (lines referenced in your
comment) so all fenced blocks have languages and all tables are separated by
blank lines.
- Around line 246-247: The doc contains a contradiction about rollout behavior
between the new /entity-graph endpoint and the existing /classes/graph shim:
update the text to be consistent by either (A) removing or adjusting the claim
that out-of-order frontend deploys won’t break due to the /classes/graph shim
(since that shim does not cover /entity-graph), or (B) explicitly document that
the /classes/graph shim only protects the classes route and that moving to
/entity-graph will allow the web to break if the API isn’t deployed first;
reference and change the sentences that mention "/entity-graph" and
"/classes/graph" and clarify the rollout guidance near the paragraphs that also
reference DeveloperEditorLayout/sourceIriIndex to keep the earlier note about no
feature flag or try/catch fallback consistent with the chosen approach.

In @.planning/STATE.md:
- Around line 6-14: The frontmatter field stopped_at currently says "Phase 17"
but the active-position/progress block still reports Phase 16 and 70%—update the
active-position and progress fields to match the frontmatter: set
active-position.phase to Phase 17, update progress.completed_phases/total_phases
and completed_plans/total_plans (or at minimum progress.percent) so the percent
reflects the 9/11 -> 92% values shown in stopped_at, and ensure any
session-continuity fields mirror stopped_at to remove the conflicting sources of
truth (fields to edit: stopped_at, active-position, and progress).
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 526c150c-6ace-467c-baef-164088151eec

📥 Commits

Reviewing files that changed from the base of the PR and between f3a17a6 and ef46639.

📒 Files selected for processing (12)
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-01-PLAN.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-02-PLAN.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-03-PLAN.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-CONTEXT.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-DISCUSSION-LOG.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-PATTERNS.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-RESEARCH.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-SPEC.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-UI-SPEC.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-VALIDATION.md
✅ Files skipped from review due to trivial changes (3)
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-UI-SPEC.md
  • .planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-DISCUSSION-LOG.md
  • .planning/ROADMAP.md

Comment on lines +7 to +12
files_modified:
- "ontokit-api: ontokit/api/routes/projects.py"
- "ontokit-api: ontokit/services/ontology.py"
- "ontokit-api: ontokit/schemas/graph.py"
- "ontokit-api: tests/unit/test_entity_graph.py"
autonomous: false
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

files_modified is missing a required test artifact

Lines 375–377 require tests/unit/test_graph_schema.py, but it is not declared in files_modified (Lines 7–12). If execution tooling enforces this list, the schema test can be skipped or flagged as out-of-scope.

Also applies to: 375-377

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-01-PLAN.md
around lines 7 - 12, The plan's metadata key files_modified is missing the
required test artifact tests/unit/test_graph_schema.py referenced later (lines
375–377); update the files_modified list to include
tests/unit/test_graph_schema.py alongside the existing entries
(ontokit/api/routes/projects.py, ontokit/services/ontology.py,
ontokit/schemas/graph.py, tests/unit/test_entity_graph.py) so the execution
tooling will include and run the schema test.

<action>
**Sub-step A — Extend `lib/stores/selectionStore.ts`** per D-13/D-14/D-15 and RESEARCH Code Example 3:

1. Add a `PaneTab` type export at the top: `export type PaneTab = "detail" | "graph" | "source";`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Convert indented code snippets to fenced code blocks

These sections use indented code blocks where markdownlint expects fenced style (MD046). Converting them to fenced blocks will keep docs lint clean and preserve syntax highlighting consistently.

Also applies to: 356-356, 416-416, 474-474, 602-602, 619-619, 771-771, 850-850, 873-873, 1120-1120

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 263-263: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-02-PLAN.md
at line 263, The markdown uses indented code blocks (e.g., the example line
adding the PaneTab type export `export type PaneTab = "detail" | "graph" |
"source";`) which triggers MD046; replace each indented snippet with a fenced
code block using triple backticks and an appropriate language tag (```ts for
TypeScript examples) so the snippet is fenced and syntax-highlighted; apply this
change to the mentioned occurrences (lines cited in the review) to ensure
consistent fenced blocks throughout the document.

Comment on lines +808 to +814
| T-17-13 | T (Tampering) | useGraphData focusType param threading | mitigate | TypeScript Literal "class"|"property"|"individual" enforces compile-time validation; default of "class" preserves behavior on missing values |
| T-17-14 | I (Information disclosure) | OntologyGraph rendered in Graph tab | accept | Read-only graph; no PII; same data already accessible via the existing /classes/graph route |
| T-17-15 | D (Denial of service) | Maximize takeover via CSS attribute toggle | accept | Single document attribute write per state transition; useEffect cleanup guarantees attribute removal on unmount; bounded transitions |
| T-17-16 | E (Elevation of privilege) | Tab persistence in selectionStore (non-persist) | accept | activePaneTab is session-only client state; no privileged action; D-14 explicitly chose non-persist to avoid stale-state attacks across reload |
| T-17-17 | T (Tampering) | EntityModal aria-labelledby titleId | mitigate | titleId is rendered as id attribute on a known element controlled by the layout; no user-supplied IDs reach this attribute |
| T-17-18 | I (Information disclosure) | Smoke test screenshots written to $HOME | mitigate | Screenshots are deleted by Task 3 Sub-step E (per CLAUDE.md global instructions); no persistent artifact in repo or home dir |
</threat_model>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Escape pipe characters in STRIDE table cell content

Line 808 includes class"|"property"|"individual inside a table cell. The unescaped pipes split columns and trigger MD056/MD055; escape them (\|) or wrap the union in backticks without raw pipes.

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 808-808: Table column count
Expected: 5; Actual: 7; Too many cells, extra data will be missing

(MD056, table-column-count)


[warning] 814-814: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing leading pipe

(MD055, table-pipe-style)


[warning] 814-814: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing trailing pipe

(MD055, table-pipe-style)


[warning] 814-814: Table column count
Expected: 5; Actual: 1; Too few cells, row will be missing data

(MD056, table-column-count)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-03-PLAN.md
around lines 808 - 814, The table cell in the T-17-13 row currently contains the
unescaped union literal class"|"property"|"individual which is breaking markdown
table parsing and triggering MD056/MD055; edit the T-17-13 cell (the
`useGraphData focusType param threading` row) and either escape the pipe
characters as class"\|"\|property"\|"\|individual or wrap the entire union in
inline code/backticks (e.g., `class|property|individual`) so the pipes are not
treated as column separators.

Comment on lines +51 to +52
- **D-01:** Snippet computed **frontend-only** from already-loaded full source. Generalize `findBlock` in `lib/ontology/turtleClassUpdater.ts` to handle property + individual subject patterns (currently class-only). Use the Web Worker IRI index from `lib/editor/indexWorker.ts` for the start line number.
- **D-02:** When the extractor cannot locate the entity (e.g., entity defined in an imported ontology, malformed source), render an **empty-state body**: `"Source not available for this entity — it may be defined in an imported ontology."` Toolbar still shows `↗ Open full source` so the user can verify in the full file.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Unify snippet-extraction contract (current text conflicts with locked phase guidance)

Line 51 and related references still direct a findBlock generalization in turtleClassUpdater.ts, but the phase pattern contract says turtleUtils.findBlock is already type-agnostic and only a wrapper should be added. Keeping both instructions risks divergent implementations.

Also applies to: 102-103, 125-126

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-CONTEXT.md
around lines 51 - 52, The comment flags conflicting guidance: don't re-implement
type-agnostic block-finding in lib/ontology/turtleClassUpdater.ts; instead adapt
the existing turtleUtils.findBlock wrapper to support both property and
individual-subject patterns and call that from turtleClassUpdater.ts; update
turtleClassUpdater.ts to use turtleUtils.findBlock (not a new findBlock) and
ensure it consumes the Web Worker IRI index from lib/editor/indexWorker.ts for
start line numbers; also adjust any references at lines ~102-103 and ~125-126 to
match the unified wrapper approach so all callers share the same type-agnostic
contract.

Comment on lines +69 to +72
- **D-13:** `activePaneTab: 'detail' | 'graph' | 'source' | null` is added to **`useSelectionStore`** (per SPEC Requirement 6 literal — "useSelectionStore gains an activePaneTab field"). Selection-related state stays colocated.
- **D-14:** **Session-only persistence** — Zustand non-persist (selectionStore is already non-persist). Page reload starts on Detail. Matches SPEC Requirement 8 literal: "first entity opened in a NEW SESSION lands on Detail."
- **D-15:** **Derived fallback at render** — components consume `useEffectiveTab(editorMode)` which returns `(activePaneTab === 'source' && editorMode === 'standard') ? 'detail' : activePaneTab`. The store value is **never mutated** by mode change. User returning to Developer view restores their Source tab preference. Most graceful fallback semantics.
- **D-16:** **No URL deep-linking** for the active tab — tab is session state, not URL state. URL stays focused on entity (`?classIri=...`, `?propertyIri=...`, `?individualIri=...`). Deep-linkable tabs deferred to v0.6+ backlog.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize activePaneTab type contract across phase docs

Line 69 allows activePaneTab to be null, but D-14/D-15 and downstream plan/test contracts assume default "detail" and reset-to-"detail" semantics. This ambiguity can produce incompatible store interfaces between plans.

🧰 Tools
🪛 LanguageTool

[grammar] ~69-~69: Ensure spelling is correct
Context: ... field"). Selection-related state stays colocated. - D-14: **Session-only persistence...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-CONTEXT.md
around lines 69 - 72, Normalize the activePaneTab contract by removing null as
an allowed value and making activePaneTab strictly 'detail' | 'graph' | 'source'
in useSelectionStore and all phase docs; ensure useSelectionStore initializes to
'detail' and that any reset logic (referenced by D-14/D-15) explicitly sets
activePaneTab = 'detail', and update useEffectiveTab(editorMode) wording to
treat activePaneTab as always present (only deriving a fallback from 'source'
when editorMode === 'standard') so downstream plans/tests assume a non-null
default.

Comment on lines +708 to +712
```
flex border-b border-slate-200 dark:border-slate-700
flex-1 px-3 py-2 text-xs font-medium transition-colors
```
Active: `border-b-2 border-primary-600 text-primary-600 dark:border-primary-400 dark:text-primary-400`. Phase 17 adds the accent-tint background `bg-primary-50 dark:bg-primary-900/20` to the existing pattern.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to this fenced code block

Line 708 opens a fence without a language, which triggers MD040 and reduces readability/syntax highlighting.

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 708-708: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-PATTERNS.md
around lines 708 - 712, The fenced code block that currently starts with the
lines containing the Tailwind utility classes (e.g., "flex border-b
border-slate-200 dark:border-slate-700" and "flex-1 px-3 py-2 text-xs
font-medium transition-colors") is missing a language identifier; update the
opening ``` fence to include a language such as css or html (e.g., ```css) to
satisfy MD040 and enable proper syntax highlighting for the utility-class
snippet.

## Standard Stack

### Core (already in repo — versions verified from `package.json` 2026-05-02)
| Library | Version | Purpose | Why Standard |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix markdownlint warnings in fenced blocks/tables.

Static analysis flags MD040/MD058 here. Please add fenced-code language identifiers and blank lines around tables to keep docs lint-clean.

Also applies to: 58-58, 64-64, 99-99, 153-153, 313-313, 480-480, 758-758, 770-770

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 42-42: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-RESEARCH.md
at line 42, The MD040/MD058 warnings are caused by missing fenced-code language
identifiers and tables lacking surrounding blank lines; add explicit language
tags (e.g., ```md or ```text) to every fenced code block and ensure there is a
blank line immediately before and after each markdown table (for example the
table starting with "| Library | Version | Purpose | Why Standard |"); apply the
same fixes to the other table occurrences noted (lines referenced in your
comment) so all fenced blocks have languages and all tables are separated by
blank lines.

Comment on lines +246 to +247
- **Don't add a feature flag or try/catch fallback for the new `/entity-graph` endpoint.** D-08: sequential cross-repo merge order — api ships first to dev, then web cuts from dev. No graceful degradation; if API isn't deployed, web breaks (and that's fine — it's a coordinated release).
- **Don't re-parse the full Turtle source for line numbers.** The Web Worker IRI index already maps every subject IRI to `{line, col, len}`. Read it from `sourceIriIndex` (already a `Map<string, IriPosition>` plumbed through `DeveloperEditorLayout` — lines 91, 152, 366).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Resolve the rollout-contract contradiction for web/API ordering.

Line 246 says web can break if API is not deployed yet (and that is acceptable), but Line 735 says out-of-order frontend deploys won’t break because of the /classes/graph shim. Those statements conflict once web migrates to /entity-graph.

Proposed doc fix
- The one cross-repo coordination is the API endpoint URL (`/classes/graph` → `/entity-graph`). D-05 mitigates by keeping `/classes/graph` as a delegating shim during the deprecation window, so even an out-of-order frontend deploy doesn't break.
+ The one cross-repo coordination is the API endpoint URL (`/classes/graph` → `/entity-graph`).
+ The `/classes/graph` shim protects older clients only.
+ New web builds that call `/entity-graph` still require API-first deployment/merge sequencing (D-08).

Also applies to: 735-736

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/17-graph-as-entity-scoped-tab-in-detail-pane/17-RESEARCH.md
around lines 246 - 247, The doc contains a contradiction about rollout behavior
between the new /entity-graph endpoint and the existing /classes/graph shim:
update the text to be consistent by either (A) removing or adjusting the claim
that out-of-order frontend deploys won’t break due to the /classes/graph shim
(since that shim does not cover /entity-graph), or (B) explicitly document that
the /classes/graph shim only protects the classes route and that moving to
/entity-graph will allow the web to break if the API isn’t deployed first;
reference and change the sentences that mention "/entity-graph" and
"/classes/graph" and clarify the rollout guidance near the paragraphs that also
reference DeveloperEditorLayout/sourceIriIndex to keep the earlier note about no
feature flag or try/catch fallback consistent with the chosen approach.

Comment on lines +79 to +83
11. **Property and individual graph backend**: The entity-graph backend supports class, property, AND individual focus IRIs.
- Current: `ontokit-api` PR #37 (`entity-graph-endpoint`) supports `focus_iri` typed as a class only. Property and individual graphs are not implemented.
- Target: The `/api/v1/projects/{id}/ontology/classes/graph` endpoint (or equivalents per type) returns BFS neighborhood for class, property, and individual focus IRIs. **For properties:** nodes include the property itself (focus), domain classes, range classes, parent properties (rdfs:subPropertyOf chain), and see-also targets; edges are labelled `domain`, `range`, `subPropertyOf`, `seeAlso`. **For individuals:** nodes include the individual itself, class assertions (rdf:type), individuals connected via object property values, see-also targets, and sameAs targets; edges labelled `rdf:type`, `<predicate>`, `seeAlso`, `sameAs`. **For annotation properties:** when no domain, range, parent, or see-also exists, the response is `{nodes: [focus_only], edges: []}` and the frontend renders an empty-state message ("No relationships to display for this annotation property").
- Acceptance: Backend endpoint returns non-empty `nodes` and `edges` for at least one object property in the test ontology with domain + range; for at least one individual with rdf:type + ≥1 object property value; for at least one annotation property with see-also. Annotation property with no relationships returns the focus-only response and frontend shows the empty-state copy.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Lock Requirement 11 to one endpoint contract (/entity-graph).

Requirement 11 is currently permissive (/classes/graph or equivalents), which can lead to split implementations. Since this phase already defines unified focus_iri + focus_type, the spec should explicitly make /entity-graph authoritative and describe /classes/graph only as a temporary deprecated shim.

Proposed spec wording
- - Target: The `/api/v1/projects/{id}/ontology/classes/graph` endpoint (or equivalents per type) returns BFS neighborhood for class, property, and individual focus IRIs.
+ - Target: The authoritative endpoint is `/api/v1/projects/{id}/ontology/entity-graph?focus_iri=...&focus_type=class|property|individual`, returning BFS neighborhoods for class, property, and individual focus IRIs. `/classes/graph` remains a deprecated delegating shim during the transition window only.

Comment thread .planning/STATE.md
Comment on lines +6 to +14
stopped_at: Phase 17 plans approved
last_updated: "2026-05-02T18:49:51.167Z"
last_activity: 2026-04-08
progress:
total_phases: 10
total_phases: 11
completed_phases: 9
total_plans: 35
total_plans: 38
completed_plans: 35
percent: 100
percent: 92
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Resolve Phase-state drift between frontmatter/session continuity and current-position sections

Line 6 and Lines 212–214 now point to Phase 17, but the active-position block still reports Phase 16 and 70% progress. This leaves two conflicting sources of truth in the same state file and can break resume handoff behavior.

Also applies to: 212-214

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/STATE.md around lines 6 - 14, The frontmatter field stopped_at
currently says "Phase 17" but the active-position/progress block still reports
Phase 16 and 70%—update the active-position and progress fields to match the
frontmatter: set active-position.phase to Phase 17, update
progress.completed_phases/total_phases and completed_plans/total_plans (or at
minimum progress.percent) so the percent reflects the 9/11 -> 92% values shown
in stopped_at, and ensure any session-continuity fields mirror stopped_at to
remove the conflicting sources of truth (fields to edit: stopped_at,
active-position, and progress).

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.

1 participant