docs: define Hill 1 semantic bootstrap slice#308
Conversation
Summary by CodeRabbit
WalkthroughDocumentation and schema updates establish "Hill 1 Semantic Bootstrap" as the explicit current focus, add a new Hill 1 design spec and docs index entry, and extend the canonical graph schema and runtime validators with a Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/h1-semantic-bootstrap.md`:
- Line 83: Update the docs line that currently reads "`doc:` for general
markdown documents" to use the proper noun capitalization — change "markdown" to
"Markdown" so it reads "`doc:` for general Markdown documents"; ensure any other
occurrences in the same file follow this "Markdown" capitalization for
consistency.
- Around line 171-177: The doc currently presents the command literal "git mind
bootstrap" as if available; update the text around the suggested command shape
(the "git mind bootstrap" example) to add a clear one-line status note such as
"Status: planned contract for Hill 1 — this command is not implemented in the
current CLI yet" so readers won't assume it's shipped; place this note
immediately after the code block where "git mind bootstrap" appears and keep the
phrasing concise and unambiguous.
In `@ROADMAP.md`:
- Around line 92-123: Update the roadmap diagram label that currently reads
"Stabilize And Clarify" to match the section heading "Current Focus: Hill 1
Semantic Bootstrap" so the document is self-consistent; locate the diagram/label
text in the ROADMAP.md content and replace that label string with "Hill 1
Semantic Bootstrap" (or equivalent phrasing used in the section header) ensuring
any alt text/captions are updated too.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: d36aa2d4-0688-4b7d-9fa2-f2827fe3279d
📒 Files selected for processing (3)
ROADMAP.mddocs/README.mddocs/design/h1-semantic-bootstrap.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ROADMAP.md`:
- Around line 92-123: Update the "Current Focus: Hill 1 Semantic Bootstrap"
block to explicitly name the sponsor user and the job-to-be-done (JTBD): add two
short fields such as "Sponsor user: <role/persona>" and "Job-to-be-done:
<concise JTBD statement>" within that section so the focus is auditable against
the operating model; edit the "Current Focus: Hill 1 Semantic Bootstrap" header
area (the block containing Status, Goal, Deliverables, Exit criteria, Primary
references) to include those two lines and ensure wording ties to the Hill
(e.g., sponsor = unfamiliar-repo maintainer or developer persona, JTBD =
"quickly understand repository structure and key entities with minimal input")
so reviewers can verify alignment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
@coderabbitai review please 🥕 |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@GRAPH_SCHEMA.md`:
- Line 360: The taxonomy and appendix are inconsistent: the constant
SYSTEM_PREFIXES lists ['commit','repo','epoch'] but Section 3’s system-generated
table documents only commit:, so unify the contract by making the authoritative
taxonomy match the appendix (or vice versa). Update the declaration referencing
SYSTEM_PREFIXES to reflect the single canonical set of system prefixes used in
Section 3 (e.g., remove or add 'repo' and 'epoch' to match the documented
table), and ensure any prose in Section 3 or the appendix is edited to state the
exact same prefixes and format (including colon suffix like "commit:") so
SYSTEM_PREFIXES and the Section 3 table are identical.
- Around line 152-153: Update the stale cardinality wording mentioning "8 valid
edge types" to reflect the current contract of eleven types (e.g., replace the
literal "8 valid edge types" with "11 valid edge types" and scan the document
for any other occurrences of the old cardinality phrase); ensure the non-example
descriptions and any text that contrasts with "Eleven directed edge types are
defined" are consistent with the updated count.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 1282c768-afe9-462a-bdf0-f871b3bd074e
📒 Files selected for processing (6)
GRAPH_SCHEMA.mdROADMAP.mddocs/design/h1-semantic-bootstrap.mdsrc/validators.jstest/edges.test.jstest/validators.test.js
|
@coderabbitai review please 🥕 |
|
✅ Actions performedReview triggered.
|
|
@coderabbitai review please 🥕 |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
GRAPH_SCHEMA.md (2)
82-86: 🧹 Nitpick | 🔵 TrivialSemantic overload:
repo:is both a "system prefix" and a cross-repo namespace qualifier.The spec lists
repo:in the System-Generated (Reserved) prefix table (line 142) alongsidecommit:andepoch:, which are actual node prefixes. Butrepo:is NOT a node prefix—it's a namespace qualifier for cross-repo IDs.Confusion:
- Line 82: "
repo:prefix is a system prefix"- Line 83: "returns the inner prefix (e.g.,
crateforrepo:owner/name:crate:id)"- So for
repo:foo/bar:task:123, the actual prefix istask:, notrepo:Impact:
- Readers see
repo:in the system prefix list and assume it works likecommit:orepoch:- But you cannot have a node
repo:123— that's invalid syntax- The SYSTEM_PREFIXES array mixes namespace qualifiers with actual prefixes
Consider one of these fixes:
Option 1: Move
repo:to a separate section "Reserved Namespaces" instead of listing it as a prefix:### Reserved Namespaces | Namespace | Purpose | Example | |-----------|---------|---------| | `repo:` | Cross-repo ID qualifier | `repo:owner/name:crate:id` |Option 2: Add a clarifying note in the system prefix table:
| `repo:` | Cross-repo qualifier namespace (not a node prefix) | `repo:owner/name:crate:echo-core` |Option 3: Keep as-is but add explicit prose: "Note: Unlike
commit:andepoch:,repo:is not a node prefix—it's a namespace qualifier that wraps other node IDs."Also applies to: 142-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GRAPH_SCHEMA.md` around lines 82 - 86, The doc incorrectly treats `repo:` as a node prefix; update the schema so `repo:` is documented as a cross-repo namespace qualifier rather than a system node prefix: change the SYSTEM_PREFIXES listing (or the System-Generated (Reserved) table) to remove or annotate `repo:` and add a separate "Reserved Namespaces" entry for `repo:` (or a clarifying note) showing examples like `repo:owner/name:crate:id`, and adjust the prose around extractPrefix to state that extractPrefix returns the inner node prefix (e.g., `crate` for `repo:owner/name:crate:id`) so readers understand `repo:` wraps, not replaces, actual prefixes.
83-83: 🛠️ Refactor suggestion | 🟠 MajorImplementation leakage: Spec references undefined
extractPrefixfunction.Line 83 states "
extractPrefixreturns the inner prefix" but never defines whatextractPrefixis, where it exists, or its signature. This is an implementation detail leaking into what should be an implementation-agnostic contract specification.Impact:
- Readers must guess that
extractPrefixis a function in the codebase- The spec fails to define the behavior independently
- Violates the stated principle that this is the "authoritative contract" (line 3) that implementations build against
Rewrite line 83 to define the behavior without referencing implementation:
-- `extractPrefix` returns the **inner** prefix (e.g., `crate` for `repo:owner/name:crate:id`) +- The prefix of a cross-repo ID is the **inner** prefix (e.g., `crate` for `repo:owner/name:crate:id`)Or add a definition:
**Term: extractPrefix(nodeId)** — Returns the prefix portion of a node ID. For cross-repo IDs, returns the inner prefix after the `repo:owner/name:` qualifier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GRAPH_SCHEMA.md` at line 83, Replace the implementation-leaning sentence mentioning "extractPrefix" with an implementation-agnostic term definition that specifies the behavior: define "extractPrefix(nodeId)" (or a "prefix" term) as "the prefix portion of a node ID; for cross-repo IDs the inner prefix is the segment after the 'repo:owner/name:' qualifier (e.g., 'crate' for 'repo:owner/name:crate:id')", so the spec stands alone without assuming a concrete function exists; reference the term "nodeId" and show the cross-repo example to clarify expected parsing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@GRAPH_SCHEMA.md`:
- Around line 165-168: Add a short "Directionality note" to Section 4 clarifying
that `groups` and `belongs-to` are inverse containment relationships and specify
a convention: use `belongs-to` for manually authored child→parent edges and
reserve `groups` for system-generated parent→child containment edges (or
explicitly state deprecation of one if preferred); reference the symbols
`groups` and `belongs-to` and update any examples in the schema to reflect the
convention so authors and queries have a single canonical direction to rely on.
- Line 143: The description of the `epoch:` prefix is too vague—update the
GRAPH_SCHEMA.md entry for `epoch:` to provide concrete semantics: define
"time-travel" as immutable, monotonic version checkpoints (snapshots) distinct
from `commit:` (where `commit:` denotes a specific change/merge event and
`epoch:` denotes broader snapshot boundaries), state when epoch nodes are
created (e.g., on snapshot, release, or compaction events), explain what they
represent (read-only snapshot ID, wall-clock and logical timestamp, and optional
metadata), and give example lifecycle rules for epoch/timeline machinery (how
epochs relate to commits, retention, and lookup). Also expand the
"epoch/timeline machinery" section to list responsibilities (create/retire
epochs, map commits to epochs, query semantics for time-travel reads) and
provide an example showing `epoch:` vs `commit:` usage and canonical formats
(hash + timestamp).
- Around line 145-150: The docs claim system prefixes (commit:, repo:, epoch:)
are rejected but the code only classifies them; update src/validators.js to
enforce this: in the import/node validation path (where classifyPrefix() and the
edge validation run) detect prefixes that match SYSTEM_PREFIXES and throw/reject
with a clear error (use classifyPrefix() to identify system prefixes), ensuring
user-authored import nodes and edges containing commit:, repo:, or epoch:
produce a hard error; add unit tests for the import validation path that assert
rejection and update any warning-only branches to raise errors instead of
silently accepting these prefixes.
---
Outside diff comments:
In `@GRAPH_SCHEMA.md`:
- Around line 82-86: The doc incorrectly treats `repo:` as a node prefix; update
the schema so `repo:` is documented as a cross-repo namespace qualifier rather
than a system node prefix: change the SYSTEM_PREFIXES listing (or the
System-Generated (Reserved) table) to remove or annotate `repo:` and add a
separate "Reserved Namespaces" entry for `repo:` (or a clarifying note) showing
examples like `repo:owner/name:crate:id`, and adjust the prose around
extractPrefix to state that extractPrefix returns the inner node prefix (e.g.,
`crate` for `repo:owner/name:crate:id`) so readers understand `repo:` wraps, not
replaces, actual prefixes.
- Line 83: Replace the implementation-leaning sentence mentioning
"extractPrefix" with an implementation-agnostic term definition that specifies
the behavior: define "extractPrefix(nodeId)" (or a "prefix" term) as "the prefix
portion of a node ID; for cross-repo IDs the inner prefix is the segment after
the 'repo:owner/name:' qualifier (e.g., 'crate' for
'repo:owner/name:crate:id')", so the spec stands alone without assuming a
concrete function exists; reference the term "nodeId" and show the cross-repo
example to clarify expected parsing behavior.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: b35c8392-b1c8-40ac-bd59-d358dfe0cb16
📒 Files selected for processing (1)
GRAPH_SCHEMA.md
| |--------|---------|---------| | ||
| | `commit:` | Git commit (auto-generated by hooks) | `commit:934b6e3` | | ||
| | `repo:` | Cross-repo qualifier namespace for remote node IDs | `repo:owner/name:crate:echo-core` | | ||
| | `epoch:` | System-generated temporal/version marker for time-travel | `epoch:934b6e3` | |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Vague description: epoch: "time-travel" semantics are undefined.
Line 143 describes epoch: as a "System-generated temporal/version marker for time-travel" but never defines:
- What "time-travel" means in this context
- How
epoch:differs fromcommit:(both examples use hash-like identifiers) - When epoch nodes are created
- What they represent (snapshots? checkpoints? version boundaries?)
The phrase "epoch/timeline machinery" (line 149) is equally opaque.
Impact:
- Readers cannot understand the purpose or semantics of epoch nodes
- Implementers have no guidance on when to create epoch: nodes
- The spec fails to define this reserved prefix clearly
Expand the description with concrete semantics:
-| `epoch:` | System-generated temporal/version marker for time-travel | `epoch:934b6e3` |
+| `epoch:` | System-generated snapshot marker for graph state at a point in time | `epoch:2026Q1-release` |And expand line 149:
-- `epoch:` nodes are created automatically by the epoch/timeline machinery and are not authored directly in import files.
+- `epoch:` nodes are created automatically when capturing graph snapshots (e.g., for versioning, rollback, or historical queries) and are not authored directly in import files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@GRAPH_SCHEMA.md` at line 143, The description of the `epoch:` prefix is too
vague—update the GRAPH_SCHEMA.md entry for `epoch:` to provide concrete
semantics: define "time-travel" as immutable, monotonic version checkpoints
(snapshots) distinct from `commit:` (where `commit:` denotes a specific
change/merge event and `epoch:` denotes broader snapshot boundaries), state when
epoch nodes are created (e.g., on snapshot, release, or compaction events),
explain what they represent (read-only snapshot ID, wall-clock and logical
timestamp, and optional metadata), and give example lifecycle rules for
epoch/timeline machinery (how epochs relate to commits, retention, and lookup).
Also expand the "epoch/timeline machinery" section to list responsibilities
(create/retire epochs, map commits to epochs, query semantics for time-travel
reads) and provide an example showing `epoch:` vs `commit:` usage and canonical
formats (hash + timestamp).
| **System prefix rules:** | ||
| - `commit:` nodes are created automatically by the post-commit hook. | ||
| - User-provided `commit:` nodes in import files are **rejected** (hard error). | ||
| - `repo:` is reserved for system-managed cross-repo qualification and should not be introduced ad hoc in import files. | ||
| - `epoch:` nodes are created automatically by the epoch/timeline machinery and are not authored directly in import files. | ||
| - Future system prefixes will use the `sys-` namespace (e.g., `sys-audit:`). |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how system prefixes are enforced in import logic
echo "=== Search for commit: validation/rejection logic ==="
rg -n "commit:" src/import.js src/validators.js -B2 -A2
echo -e "\n=== Search for system prefix enforcement ==="
rg -n "SYSTEM_PREFIXES|system.*prefix|classifyPrefix" src/ -B2 -A2 | head -50
echo -e "\n=== Search for hard error conditions ==="
rg -n "hard error|reject.*prefix|system.*reject" src/ -B1 -A1Repository: flyingrobots/git-mind
Length of output: 2539
Documentation falsely promises system prefix enforcement that doesn't exist in code.
Lines 147-149 claim system prefixes are controlled:
commit:nodes are "rejected (hard error)"repo:"should not be introduced ad hoc"epoch:"are not authored directly"
None of this is enforced. The code has zero runtime protection against any of these prefixes appearing in user-authored nodes.
Evidence from src/validators.js:
classifyPrefix()(lines 93-96) is a read-only classifier, not an enforcer—it returns classification strings but takes no action- Edge validation (lines 186-196) only warns about unknown prefixes; system prefixes are silently accepted
- The
SYSTEM_PREFIXESarray (line 27) documents what exists, but no code rejects or errors on them
Result: Users CAN currently create commit:, repo:, and epoch: nodes in import files without any error, warning, or rejection, directly contradicting what the spec promises.
Required: Either enforce the restrictions with actual rejection logic, or remove the false claims from the documentation. Claiming "hard error" and "rejected" while the code accepts them is a contract violation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@GRAPH_SCHEMA.md` around lines 145 - 150, The docs claim system prefixes
(commit:, repo:, epoch:) are rejected but the code only classifies them; update
src/validators.js to enforce this: in the import/node validation path (where
classifyPrefix() and the edge validation run) detect prefixes that match
SYSTEM_PREFIXES and throw/reject with a clear error (use classifyPrefix() to
identify system prefixes), ensuring user-authored import nodes and edges
containing commit:, repo:, or epoch: produce a hard error; add unit tests for
the import validation path that assert rejection and update any warning-only
branches to raise errors instead of silently accepting these prefixes.
| | `groups` | Source groups or contains target | parent → child | `module:auth → file:src/auth.js` | | ||
| | `blocks` | Source blocks progress on target | blocker → blocked | `task:BDK-001 → task:BDK-002` | | ||
| | `belongs-to` | Source is a child/member of target | child → parent | `feature:BDK-SCHEMA → milestone:BEDROCK` | | ||
| | `consumed-by` | Source is consumed/used by target | resource → consumer | `pkg:chalk → module:format` | |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Semantic overlap: groups and belongs-to are inverse relationships with no disambiguation guidance.
Line 165 defines groups as "Source groups or contains target" (parent → child), while line 167 defines belongs-to as "Source is a child/member of target" (child → parent). These are semantic inverses of the same relationship.
Impact:
- Users must choose between
module:auth --groups--> file:src/auth.jsandfile:src/auth.js --belongs-to--> module:auth - The spec provides zero guidance on which direction to prefer
- Graph queries will need to check both directions to find containment relationships
- Inconsistent usage across the codebase will fragment the knowledge graph
Add a guidance note to Section 4 clarifying when to use each direction, or consider deprecating one in favor of the other. For example:
**Directionality note:** `groups` and `belongs-to` are inverse relationships. By convention, use `belongs-to` when authoring manual edges (child → parent), and reserve `groups` for system-generated containment edges (parent → child).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@GRAPH_SCHEMA.md` around lines 165 - 168, Add a short "Directionality note" to
Section 4 clarifying that `groups` and `belongs-to` are inverse containment
relationships and specify a convention: use `belongs-to` for manually authored
child→parent edges and reserve `groups` for system-generated parent→child
containment edges (or explicitly state deprecation of one if preferred);
reference the symbols `groups` and `belongs-to` and update any examples in the
schema to reflect the convention so authors and queries have a single canonical
direction to rely on.
1 participant
PR Title
docs: define Hill 1 semantic bootstrap slice
Summary
docs/design/h1-semantic-bootstrap.md#304through#307from the accepted first-slice framingProblem Statement
Git Mind's product frame and planning model were clarified, but the repo still lacked a concrete first-slice spec for the actual product move that matters: point Git Mind at an unfamiliar repository and get a useful first semantic map with low manual input. Without that spec, the backlog remained tilted toward supporting work while the core Hill 1 product slice stayed underspecified.
ADR Compliance (Required)
Relevant ADR(s)
Compliance Declaration
Exception Request (Required if deviating)
Architecture Laws Checklist (Hard Gates)
Canonical Truth & Context
--at,--observer,--trust) or deterministically defaulted.Determinism & Provenance
Artifact Hygiene
Contracts & Compatibility
Extension/Effects Safety (if applicable)
Scope Control
Backward Compatibility
Test Plan (Required)
Unit
# Docs/spec PR; no unit test changes.Integration
# Docs/spec PR; no integration test changes.Determinism
# Not applicable for this docs/spec change.Contract/Schema
# No schema changes.Policy Gates
Security / Trust Impact
Performance Impact
Observability / Debuggability
Operational Notes
npm testsuite continues to exhibit pre-existing timeout flake under full load, so I did not use it as the primary validation signal for this change.Linked Issues / Milestones
Reviewer Quick Verdict Block (for maintainers)
MUST (Hard Gates)
SHOULD (Quality)
Verdict