okf: recommend relative cross-links over absolute (§5.1/§5.2) - #165
okf: recommend relative cross-links over absolute (§5.1/§5.2)#165jeromeetienne wants to merge 5 commits into
Conversation
SPEC §5.1 recommended /-prefixed absolute (bundle-relative) links, but these only resolve in an OKF-aware consumer. A raw renderer resolves a leading / against its own root (host origin in a browser, repo root on GitHub), which is not the bundle root once the bundle is nested as a subdirectory within a larger repository (allowed by §3), so the links mislink or 404. The reference implementation had already voted against the spec: the agent prompt forbids leading /, and every cross-link in the shipped bundles is relative. This aligns the spec with that reality. - Swap §5.1/§5.2 so relative links are §5.1 and marked recommended. - Reframe absolute links as §5.2, documenting that they require an OKF-aware resolver and the browser/GitHub/nested-subdir failure mode. - Update the §4.3/§4.4 examples to use the recommended relative form.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
+1. No objection to recommending relative links, but the new §5.2 wording is easy to read as "optional to implement", and that's already the reality: the reference visualizer silently skips leading-/ links, so a conformant bundle renders with zero edges and no warning (#48; open fixes in #66, #110, #184, #206). Please pair the swap with an explicit consumer requirement, e.g. "Consumers MUST resolve this form against the bundle root", so existing bundles that use absolute links keep working across tools. |
…root Addresses review feedback on GoogleCloudPlatform#165: the swap to recommending relative links (§5.1) left §5.2 descriptive rather than normative, so a consumer could read absolute-link support as optional. The reference visualizer already does this — it silently skips leading-/ links instead of resolving or warning (GoogleCloudPlatform#48), so a conformant bundle using absolute links can render with zero edges and no diagnostic. Add an explicit MUST-level requirement: consumers that resolve cross-links must resolve §5.2 links against the bundle root and must surface an unresolved one the same way they surface a broken §5.1 link, rather than dropping it silently.
There was a problem hiding this comment.
Good catch, pushed jeromeetienne@f87f8d3 which adds an explicit MUST-level requirement for the absolute form (now §6.1):
Consumers that resolve cross-links MUST resolve an absolute link against the bundle root, not against their own root. A consumer MUST NOT silently drop or ignore an absolute link it cannot resolve. It MUST surface the unresolved link in the same way it surfaces a broken relative link.
That should close the "optional to implement" reading. The reference visualizer's silent-skip behavior itself is tracked separately in #48 (with fixes in #66, #110, #184, #206), this PR fixes the spec wording; those issues will bring the reference implementation into compliance with it.
Records the real second parent against upstream/main. The earlier commit 70bfae7 ("Merge upstream OKF v0.2 changes") copied the upstream v0.2 tree in as a single-parent commit, so the last common ancestor with upstream stayed at d44368c and GoogleCloudPlatform#165 replayed every file changed by the upstream v0.2 migration (GoogleCloudPlatform#227) as if it were part of this pull request. With the second parent recorded, the common ancestor moves to the current upstream tip and GoogleCloudPlatform#165 shows only its own change to okf/SPEC.md. Conflict in okf/SPEC.md resolved in favour of this branch: that file is the substance of GoogleCloudPlatform#165, recommending relative cross-links over absolute. No file content changes as a result of this merge.

Fixes #157
Problem
SPEC §5.1 recommended
/-prefixed absolute (bundle-relative) links, butthese only resolve correctly in an OKF-aware consumer. A raw renderer resolves
a leading
/against its own root — the host origin in a browser, the reporoot on GitHub — which is not the bundle root once the bundle is nested as a
subdirectory within a larger repository (which §3 explicitly allows). In that
case the links mislink or 404, cutting against OKF's zero-tooling portability
goal.
The reference implementation had already voted against the spec on this:
reference_instruction.md) forbids leading/("Never start a link with
/(that breaks GitHub rendering)").bundles/*/concept files is relative.So the spec recommended absolute → the agent forbade absolute → the bundles use
relative. The only thing out of step was the "recommended" label in the spec.
Change
Adopt resolution (1) from the issue — recommend relative links, keep absolute
as a supported OKF-aware-only form:
with the rationale that they resolve in any standard renderer (
cat, browser,GitHub, editor preview) without OKF tooling. Noted drawback: breaks if a
document moves.
OKF-aware resolver and spelling out the browser/GitHub/nested-subdirectory
failure mode.
so the spec's own examples model the guidance.
Spec-text only — no code changes; this aligns the spec with what the reference
agent and bundles already do.