diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 815c4f8..86e5a9d 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -40,13 +40,13 @@ {"id":"ifcurl-gy8","title":"Viewer: 'View in 3D' uses commit hash but no UI affordance to switch to branch tip","description":"The 'View in 3D' button in footer.tmpl always generates an ifc:// URL with a commit hash (@abc123...). This is correct for reproducibility, but users who want to track 'the latest version of main' must manually change the URL from @\u003chash\u003e to @heads/main. There is no indication in the viewer that the model is pinned to a specific commit. Add a visual indicator (e.g. 'commit abc1234 · switch to heads/main') in the viewer toolbar when the ref is a commit hash, offering a one-click switch to the branch tip. The branch name could be inferred from Forgejo's API given the commit hash.","status":"open","priority":3,"issue_type":"feature","owner":"bruno@postle.net","created_at":"2026-04-24T11:04:58Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:04:58Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-dag","title":"Viewer: no model metadata panel — disciplines, spatial structure, element counts","description":"Beyond individual element selection, reviewers and designers need a model overview: what disciplines/types are present, how many elements, what building storeys exist, what's the spatial structure (site/building/storey/space hierarchy). A collapsible side panel showing IFC spatial tree and type breakdown would give immediate context and allow navigating by storey or discipline. Without this, reviewers must already know the model structure before opening the viewer.","status":"open","priority":3,"issue_type":"feature","owner":"bruno@postle.net","created_at":"2026-04-24T11:04:55Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:04:55Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-ov6","title":"Viewer selector only supports type names — property filters require server round-trip","description":"The viewer's applySelector() only supports IFC type names and their unions ('IfcWall+IfcSlab'). Full IfcOpenShell selector syntax — filter by property value (Pset_WallCommon.FireRating=2HR), by GlobalId, by name (.Name='Core Wall'), by spatial containment — is not supported. For collaboration, reviewers need to select specific elements ('all walls with fire rating 2HR that aren't compliant') not just all walls. The service's /preview endpoint already handles full selector syntax server-side. The viewer should forward complex selectors to the service and use the rendered image, or fetch the matching GlobalIds from a new /resolve-selector service endpoint and apply them in the viewer.","status":"open","priority":3,"issue_type":"feature","owner":"bruno@postle.net","created_at":"2026-04-24T11:04:38Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:04:38Z","dependency_count":0,"dependent_count":0,"comment_count":0} -{"id":"ifcurl-f3p","title":"footer.tmpl: 'View in 3D' button absent from file history (git log) page","description":"The 'View in 3D' button is injected on the file view page (showing one commit of a file). It is not shown on the file history page (the list of commits that touched a file), which is where users naturally go to see how a model evolved over time. Each entry in the history list already has a commit hash in its href. Injecting a 'View in 3D' link or button alongside each history entry would let reviewers quickly compare model states across commits — which is a primary collaboration use case.","status":"open","priority":3,"issue_type":"feature","owner":"bruno@postle.net","created_at":"2026-04-24T11:04:23Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:04:23Z","dependency_count":0,"dependent_count":0,"comment_count":0} +{"id":"ifcurl-f3p","title":"footer.tmpl: 'View in 3D' button absent from file history (git log) page","description":"The 'View in 3D' button is injected on the file view page (showing one commit of a file). It is not shown on the file history page (the list of commits that touched a file), which is where users naturally go to see how a model evolved over time. Each entry in the history list already has a commit hash in its href. Injecting a 'View in 3D' link or button alongside each history entry would let reviewers quickly compare model states across commits — which is a primary collaboration use case.","status":"closed","priority":3,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T11:04:23Z","created_by":"Bruno Postle","updated_at":"2026-04-24T14:04:56Z","started_at":"2026-04-24T13:21:51Z","closed_at":"2026-04-24T14:04:56Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-4rj","title":"Viewer: no branch/ref switcher — can't navigate model history or compare versions","description":"The viewer loads a single ifc:// URL and has no UI to switch to a different ref. A reviewer wanting to see 'what did this look like on main before the PR' or 'show me the v1.2 tag' must manually edit the URL. For a collaboration tool, being able to flip between 'current PR branch' and 'base branch' at the same camera view is essential for review. Consider a ref dropdown or input in the toolbar that reconstructs the URL with a different @ref, preserving camera/clip/selector state. The structured fields already expose repo/ref/path — the ref field is editable but not prominently labelled as a version picker.","status":"open","priority":3,"issue_type":"feature","owner":"bruno@postle.net","created_at":"2026-04-24T11:04:20Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:04:20Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-np8","title":"Viewer depends on CDN (unpkg, esm.sh) — broken in air-gapped or restricted networks","description":"viewer.html loads all JavaScript from public CDNs: three.js from esm.sh, @thatopen/components and fragments from esm.sh/unpkg, web-ifc WASM from unpkg, the fragment worker from unpkg, JSZip from esm.sh. In corporate BIM environments with restricted internet (common in construction/engineering), the viewer will fail to load entirely. There is no documented self-hosted path. Consider: document how to serve the dependencies from the Forgejo instance's own /assets/ directory, or provide a bundled viewer.html as a build artifact. The viewer worker fetch ('https://unpkg.com/@thatopen/fragments@3.3.1/dist/Worker/worker.mjs') is already fetched at runtime with a special workaround for Firefox — this is the most fragile CDN dependency.","status":"open","priority":3,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T11:04:04Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:04:04Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-sa0","title":"Viewer: no load progress — large IFC files appear to hang","description":"The viewer status shows 'Fetching IFC file…' and 'Parsing IFC…' as plain text but gives no indication of how far along the download or parse is. IFC files in real projects are often 20–200MB. The fetch() call and the web-ifc WASM parser both block for seconds with no visual feedback. A progress bar or percentage indicator during fetch (using response.body ReadableStream + Content-Length header) and a spinner during parse would prevent users abandoning the page thinking it crashed. The status element exists and is already used — it just needs richer updates.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T11:03:58Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:47:23Z","started_at":"2026-04-24T11:14:56Z","closed_at":"2026-04-24T11:47:23Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} -{"id":"ifcurl-4xl","title":"viewer.html: selector support is a subset of IfcOpenShell syntax — not documented","description":"applySelector() in the viewer only handles type-name union syntax ('IfcWall+IfcSlab'). The comment says 'Attribute/property filters not yet supported', but this limitation is not visible to users and not documented in README.md or SPECIFICATION.md. A user pasting a URL with 'selector=IfcWall,+Pset_WallCommon.FireRating=2HR' will see all elements rather than the filtered set, with no error. Should add a user-visible note or at minimum document the viewer's selector limitations.","status":"open","priority":3,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:49Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:41:49Z","dependency_count":0,"dependent_count":0,"comment_count":0} +{"id":"ifcurl-4xl","title":"viewer.html: selector support is a subset of IfcOpenShell syntax — not documented","description":"applySelector() in the viewer only handles type-name union syntax ('IfcWall+IfcSlab'). The comment says 'Attribute/property filters not yet supported', but this limitation is not visible to users and not documented in README.md or SPECIFICATION.md. A user pasting a URL with 'selector=IfcWall,+Pset_WallCommon.FireRating=2HR' will see all elements rather than the filtered set, with no error. Should add a user-visible note or at minimum document the viewer's selector limitations.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:49Z","created_by":"Bruno Postle","updated_at":"2026-04-24T14:04:57Z","started_at":"2026-04-24T13:22:10Z","closed_at":"2026-04-24T14:04:57Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-fu5","title":"service.py: no rate limiting or request size cap on /preview and /bcf","description":"The preview and BCF endpoints have no rate limiting, no cap on IFC file size fetched, and no timeout on the render operation. A single large IFC file or a flood of requests can saturate memory and CPU. Consider: max IFC blob size (reject fetch if commit object too large), render timeout via threading.Timer, and a simple in-process rate limiter or documentation that a reverse proxy (nginx) should front the service with rate limits.","status":"open","priority":3,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:45Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:41:45Z","dependency_count":0,"dependent_count":0,"comment_count":0} -{"id":"ifcurl-d58","title":"ifc_url_test.go: no test for bare URL with path containing @ character","description":"The Go transformer reassembles multi-sibling text nodes for bare ifc:// URLs. There is a test for underscores in the path (which goldmark splits on), but no test for paths or query values containing '@', which is a valid character in query strings and could appear in selector expressions. Also missing: test for a bare URL followed immediately by punctuation like a period or closing parenthesis to verify the terminator logic.","status":"open","priority":3,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:32Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:41:32Z","dependency_count":0,"dependent_count":0,"comment_count":0} +{"id":"ifcurl-d58","title":"ifc_url_test.go: no test for bare URL with path containing @ character","description":"The Go transformer reassembles multi-sibling text nodes for bare ifc:// URLs. There is a test for underscores in the path (which goldmark splits on), but no test for paths or query values containing '@', which is a valid character in query strings and could appear in selector expressions. Also missing: test for a bare URL followed immediately by punctuation like a period or closing parenthesis to verify the terminator logic.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:32Z","created_by":"Bruno Postle","updated_at":"2026-04-24T14:04:56Z","started_at":"2026-04-24T13:22:09Z","closed_at":"2026-04-24T14:04:56Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-7mg","title":"auth.py: token appears in git process arguments, visible in process list","description":"inject_token() embeds the authentication token into the HTTPS remote URL as 'https://\u003ctoken\u003e@host/path'. This URL is passed directly to git clone/fetch as a command-line argument, which means the token is visible in the process list (ps aux) while the git command runs. On a shared server this is a security leak. The standard mitigation is to use GIT_ASKPASS or a credential helper that supplies the token via stdin/environment rather than via the URL. Should be documented as a known limitation at minimum.","status":"open","priority":3,"issue_type":"bug","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:29Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:41:29Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-z4f","title":"render.py: concurrent requests each spawn full CPU-count process pools","description":"render() calls ifcopenshell.geom.iterator(settings, model, multiprocessing.cpu_count(), ...). In a multi-request service scenario, N concurrent renders each spawn cpu_count worker processes, so a 16-core machine handling 4 simultaneous requests spawns 64 processes. This can saturate the machine and is effectively a DoS amplifier. Consider capping the worker count (e.g. max 2 or 4 per request), or using a shared process pool at the service level.","status":"open","priority":3,"issue_type":"bug","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:16Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:41:16Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"id":"ifcurl-5or","title":"README.md: library usage example doesn't show temp file pattern for ifcopenshell.open()","description":"The 'Use as a library' section shows 'model = ifcopenshell.open(...) # write ifc_bytes to tmp file first' with a comment but no actual code. ifcopenshell.open() requires a file path, not bytes. A new user following the README will be stuck. The example should show the tempfile.mkstemp pattern used in _load_model() in service.py, or at least reference that function.","status":"open","priority":3,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:04Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:41:04Z","dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/README.md b/README.md index 1c018fa..091fd4b 100644 --- a/README.md +++ b/README.md @@ -221,7 +221,10 @@ No build step — loads dependencies from CDN. - Toolbar with raw ifc:// URL input and structured fields (repo, ref, path, selector) — editing any field reloads the model -- Selector filtering — `IfcWall`, `IfcWall+IfcSlab`, etc. +- Selector filtering — `IfcWall`, `IfcWall+IfcSlab` (type-name union only in the + browser viewer; full IfcOpenShell attribute/property filters are applied + server-side by the preview service and are reflected in the URL but not + applied visually in the viewer) - Clipping planes — `✂ clip` button, double-click on model surface to place; drag handles to adjust; planes serialised back into the URL - FOV control, camera sync — the URL in the browser bar always reflects the diff --git a/forgejo/custom/public/assets/viewer.html b/forgejo/custom/public/assets/viewer.html index 38e840e..364eb29 100644 --- a/forgejo/custom/public/assets/viewer.html +++ b/forgejo/custom/public/assets/viewer.html @@ -147,7 +147,9 @@
+ placeholder="IfcWall (viewer: type names only)" + title="Type-name union: IfcWall, IfcWall+IfcSlab. Property/attribute filters are applied server-side only." + spellcheck="false">
diff --git a/forgejo/modules/markup/markdown/ifc_url.go b/forgejo/modules/markup/markdown/ifc_url.go index ed424b1..2813077 100644 --- a/forgejo/modules/markup/markdown/ifc_url.go +++ b/forgejo/modules/markup/markdown/ifc_url.go @@ -51,7 +51,7 @@ func (n *ifcURLNode) Dump(src []byte, level int) { type ifcURLTransformer struct{} // urlTerminators end a bare ifc:// URL in inline text. -const urlTerminators = " \t\n\r<>\"'" +const urlTerminators = " \t\n\r<>\"')" func (t *ifcURLTransformer) Transform(doc *ast.Document, reader text.Reader, _ parser.Context) { if setting.IfcURL.PreviewServiceURL == "" { diff --git a/forgejo/modules/markup/markdown/ifc_url_test.go b/forgejo/modules/markup/markdown/ifc_url_test.go index 3f85953..0751ef9 100644 --- a/forgejo/modules/markup/markdown/ifc_url_test.go +++ b/forgejo/modules/markup/markdown/ifc_url_test.go @@ -86,6 +86,33 @@ func TestIfcURL_InlineAmongText(t *testing.T) { assert.Contains(t, out, "for details.") } +func TestIfcURL_BareURLWithAtInQueryValue(t *testing.T) { + // @ in a query-string value must not truncate the URL; verify the full + // selector value appears in the figcaption (raw URL, HTML-escaped). + md := newTestMd(t) + out := convert(t, md, "ifc://example.com/org/repo@HEAD?path=m.ifc&selector=abc@def") + assert.Contains(t, out, `
`) + assert.Contains(t, out, `selector=abc@def`) +} + +func TestIfcURL_BareURLTerminatesAtClosingParen(t *testing.T) { + md := newTestMd(t) + out := convert(t, md, "(ifc://example.com/org/repo@HEAD?path=m.ifc) more text") + assert.Contains(t, out, `
`) + // The closing ) and trailing text must appear outside the figure + assert.Contains(t, out, ") more text") + // The URL in the preview must not contain the closing paren + assert.NotContains(t, out, "m.ifc)") +} + +func TestIfcURL_BareURLFollowedByPeriod(t *testing.T) { + md := newTestMd(t) + out := convert(t, md, "See ifc://example.com/org/repo@HEAD?path=m.ifc. Details follow.") + assert.Contains(t, out, `
`) + // Period and trailing text must remain in the output + assert.Contains(t, out, "Details follow.") +} + func TestIfcURL_ServiceTokenInPreviewURL(t *testing.T) { setting.IfcURL.PreviewServiceURL = "http://localhost:8000" setting.IfcURL.ServiceToken = "mytoken123" @@ -95,7 +122,7 @@ func TestIfcURL_ServiceTokenInPreviewURL(t *testing.T) { }) md := goldmark.New(goldmark.WithExtensions(markdown.NewIfcURLExtension())) out := convert(t, md, "[label](ifc://example.com/org/repo@HEAD?path=model.ifc)") - assert.Contains(t, out, `&token=mytoken123`) + assert.Contains(t, out, `&token=mytoken123`) } func TestIfcURL_NoTokenInPreviewURLWhenUnset(t *testing.T) { diff --git a/forgejo/templates/custom/footer.tmpl b/forgejo/templates/custom/footer.tmpl index dce5728..b413613 100644 --- a/forgejo/templates/custom/footer.tmpl +++ b/forgejo/templates/custom/footer.tmpl @@ -2,31 +2,16 @@ // Copyright 2026 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT // -// ifcurl — inject "View in 3D" button on .ifc file view pages. +// ifcurl — inject "View in 3D" button on .ifc file view and history pages. // // Deploy to: /etc/forgejo/templates/custom/footer.tmpl // -// Uses the permalink href (always a full commit hash) to construct an -// unambiguous ifc:// URL regardless of branch names containing slashes. +// Uses commit-hash hrefs (always unambiguous) to construct ifc:// URLs. (function () { "use strict"; var VIEWER_BASE = "/assets/viewer.html"; - // ------------------------------------------------------------------------- - // Find a commit-based href on this page. - // Case 1: viewing at a branch/tag — permalink button is present. - // Case 2: already viewing at a commit — current URL contains /src/commit/. - // ------------------------------------------------------------------------- - function commitHref() { - var a = document.querySelector('a[href*="/src/commit/"]'); - if (a) return a.getAttribute("href"); - if (/\/src\/commit\//.test(window.location.pathname)) { - return window.location.pathname; - } - return null; - } - // Parse /owner/repo/src/commit/{40hex}/{treepath} // Returns {repoPath, hash, treePath} or null. function parseCommitHref(href) { @@ -35,31 +20,49 @@ return { repoPath: m[1], hash: m[2], treePath: m[3] }; } + function makeViewerLink(info, label, cls) { + var host = window.location.host; + var ifcUrl = "ifc://" + host + "/" + info.repoPath + + "@" + info.hash + + "?path=" + encodeURIComponent(info.treePath); + var a = document.createElement("a"); + a.href = VIEWER_BASE + "?url=" + encodeURIComponent(ifcUrl); + a.target = "_blank"; + a.rel = "noopener noreferrer"; + a.className = cls || "ui mini basic button"; + a.textContent = label; + return a; + } + function init() { - var href = commitHref(); - if (!href) return; + var host = window.location.host; - var info = parseCommitHref(href); - if (!info) return; - if (!info.treePath.toLowerCase().endsWith(".ifc")) return; - - var host = window.location.host; - var ifcUrl = "ifc://" + host + "/" + info.repoPath - + "@" + info.hash - + "?path=" + encodeURIComponent(info.treePath); - var viewUrl = VIEWER_BASE + "?url=" + encodeURIComponent(ifcUrl); - - // Insert button alongside the existing Raw / Permalink / History buttons. + // ----------------------------------------------------------------------- + // Case 1: single file view — permalink button in .file-header-right. + // ----------------------------------------------------------------------- var btnGroup = document.querySelector(".file-header-right .ui.buttons"); - if (!btnGroup) return; + if (btnGroup) { + // Permalink href or current URL when already at a commit. + var a = document.querySelector('a[href*="/src/commit/"]'); + var href = a ? a.getAttribute("href") : window.location.pathname; + var info = parseCommitHref(href); + if (info && info.treePath.toLowerCase().endsWith(".ifc")) { + btnGroup.appendChild(makeViewerLink(info, "View in 3D")); + } + return; + } - var btn = document.createElement("a"); - btn.href = viewUrl; - btn.target = "_blank"; - btn.rel = "noopener noreferrer"; - btn.className = "ui mini basic button"; - btn.textContent = "View in 3D"; - btnGroup.appendChild(btn); + // ----------------------------------------------------------------------- + // Case 2: file history page — inject a "3D" link next to each commit's + // browse-file link (/src/commit/{hash}/{treepath}). + // ----------------------------------------------------------------------- + document.querySelectorAll('a[href*="/src/commit/"]').forEach(function(a) { + var info = parseCommitHref(a.getAttribute("href")); + if (!info || !info.treePath.toLowerCase().endsWith(".ifc")) return; + var btn = makeViewerLink(info, "3D"); + btn.style.marginLeft = "4px"; + a.insertAdjacentElement("afterend", btn); + }); } if (document.readyState === "loading") {