Repository navigation
kb init: let L1 accept new-directory indexes in bootstrap mode, and check index links - #243
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48e665178e
ℹ️ 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".
| found: list[tuple[str, tuple[int, ...]]] = [ | ||
| (target_of(m), (m.start(),)) for m in _MD_LINK.finditer(text) if keep(m.start())] |
There was a problem hiding this comment.
When a generated index contains  or ![rules][ref], these regex matches are included in the certain=True result because matching begins at ] or [. Both _index_created and check_index_links consequently treat the referenced page as linked even though Markdown renders an image rather than a navigable index link, allowing bootstrap to accept an unusable index. Exclude inline and reference images from the navigation set, while optionally retaining them for resolution checks.
Useful? React with 👍 / 👎.
| for raw, positions in found: | ||
| target = raw.split("#", 1)[0] | ||
| if not target or "://" in target or target.startswith("mailto:"): | ||
| continue | ||
| resolved = ".." if target.startswith("/") else \ | ||
| posixpath.normpath(posixpath.join(directory, target)) # absolute: never inside |
There was a problem hiding this comment.
For a changed index containing a valid external destination such as tel:+15551234567, urn:isbn:..., or //example.com/path, this filter does not classify the destination as a URL. It is instead joined to the index directory and reported as index_link_broken or index_link_escapes, contradicting the documented behavior that URLs are ignored and blocking otherwise valid bootstrap output. Detect URI schemes generically and handle network-path references rather than limiting the exception to :// and lowercase mailto:.
Useful? React with 👍 / 👎.
…x links
kb init (design v3 §9.2/§9.3) must create repos/<repo>/_index.md and
components/<group>/_index.md, which check_changeset refuses outright
("index files are never created or deleted by a change set").
check_changeset gains a keyword `bootstrap` (default False, behaviour
unchanged). When set it relaxes exactly two rules:
- an _index.md may be created in a directory that had no files at base;
it must link every page and child-directory index in its directory, and an
existing parent index must link it. Creating an index in an existing
directory and deleting any index stay refused.
- the shared knowledge/repos/_index.md may be modified (listing a new
repository); adding or deleting it stays refused.
check_index_links(base, head) is the init-side check for index edits that L1
only classifies as prose (tables, cross-directory links): links a change adds
must resolve inside the knowledge tree, a changed index keeps its base links,
and every new page or child index is linked from its parent index.
A test pins that only kb_service/init_stages.py (kb init, a later PR) may pass
bootstrap=True.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two gaps in check_index_links' navigation side (coverage and preservation),
found by the integration gate:
- Indented code (4+ columns, tab included; an indented list item counts only
inside a real list), HTML comments, HTML blocks and fences (any "hazard"
line before a link: a fence, a line whose content after container
prefixes starts with "<", a tag or comment left open) no longer count as
visible. A page linked only from there is unlinked, and moving an existing
link there is a drop.
- A navigation link must be whole: an unescaped, non-image "[" opening a
non-blank label, then "(target)". "cards](x)", "\[cards](x)", "[](x)" and
"" no longer satisfy required links or preservation, nor does a
link with a backtick or "<" in its text, or a tag or comment left open
before it on its line (a code span, comment or attribute swallows it). Reference
usages follow the same rule.
Resolution checks are unchanged (they still read every "](" form), so the
split stays fail-closed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…chers check_index_links hand-parsed Markdown to decide which links render, and every review round found another construct it misread (reference definitions inside a paragraph, escaped quotes in titles, HTML blocks, code spans...). That loop does not converge, so the links now come from markdown-it-py's CommonMark parser with GitHub pipe tables: every link_open href (not images, nor anything in their alt text), split as a URL first and then only the path percent-decoded (an encoded "#" or ":" stays part of the name). The parser settles reference definitions, titles, escapes, code spans and blocks, HTML blocks and comments the way renderers do. - resolution: links the change adds must resolve in head and stay inside the knowledge tree (base links are not re-judged; URLs/anchors are skipped) - preservation: every base link (by resolved path) must still be a head link, so moving a link into code, a comment or an HTML block drops it - coverage: new pages and child indexes must be among the parent index's links (bootstrap index creation uses the same links) - index_link_unparsed is gone: the parser defines what is a link markdown-it-py>=3 is declared in pyproject (it was only a transitive dependency of rich) and imported lazily. The visibility heuristics, the hazard/tag scanners and their regexes are deleted; behavioural regression tests are ported to what CommonMark renders, including both new repros. Real tree: appending a line to, and linking a new page from, each of the 142 indexes gives 0 issues and 0 missed links. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A link such as [Knowledge](../..) from repos/<repo>/_index.md resolves to the knowledge root, which check_index_links looked up among head's directories and never found (top-level files have dirname "", the link resolves to "."), so it reported index_link_broken for a valid link. The root is now a directory every tree has; a link above it still escapes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d70574e to
6b070cc
Compare
Part 2/9 of kb init (design v3, sol-approved:
kb-init-design.md§9.2 / §9.3).Summary
kb initmust createrepos/<repo>/_index.mdandcomponents/<group>/_index.md, which L1 refuses outright ("index files are never created or deleted by a change set").check_changeset(..., bootstrap=False): the default is byte-for-byte the current behaviour. Withbootstrap=True, exactly two rules relax:_index.mdmay be created in a directory that had no files at base. It must link every page and child-directory index in its directory, and an existing parent index must link it. Creating an index in an existing directory, or deleting any index, stays refused.knowledge/repos/_index.mdmay be modified to list a new repository (design §14; needed for the empty-KB pilot). Adding or deleting it stays refused.A test pins that the only module allowed to pass
bootstrap=Trueiskb_service/init_stages.py(kb init, a later PR). The service gate, calibration and the publisher's verifier never pass it.check_index_links(base, head)is the init-side check for index edits L1 only classifies as prose (tables, cross-directory links,general/seed links). It is fail-closed and doesn't try to be a Markdown parser:index_link_broken,index_link_escapes,index_link_unparsed) looks at every link-like text, including anything that might sit in code, and at every definition of a used reference label. Links the base already had aren't re-judged, because existing indexes link intodoc/.index_unlinked,index_missing) counts only certainly visible links. That means no fence-like line at or before the link, and no backtick before it in its block. A block here is a paragraph, list item, heading, quote, or a table-body cell of a real table (a header plus delimiter row, starting after a blank line).index_link_dropped): a base link is kept only if it is certainly visible in head, or every block it sits in appears unchanged in head after the same fence-like lines. Moving a link into code counts as dropping it.Probe: I appended a line to each of the 142 real indexes in
knowledge/, and got 0 issues.Tests
test/test_kb_init_l1.py(26 tests):repos/_index.mdwhitelist is edit-only--strict, knowledge linters): all green.🤖 Generated with Claude Code