FIxes

PLAN.md

@@ -118,6 +118,88 @@ Add `ifc://` link detection and preview embedding to Gitea.
 
 ---
 
+## Phase 3b — Web IFC viewer
+
+Add a 3D viewer that opens in a separate browser window when viewing an `.ifc`
+file in Forgejo.
+
+### Architecture
+
+- **Forgejo Go patch**: when rendering the file view for a `.ifc` file, inject
+  a "View in 3D" button that opens
+  `http://localhost:8000/viewer?url=ifc://...` in a new tab.  The ifc:// URL
+  encodes the current host, repo, ref, and file path.
+
+- **`/viewer` endpoint** in the ifcurl preview service: returns a self-contained
+  HTML page.  No server-side IFC processing — the browser does all rendering.
+
+- **Browser rendering**: the viewer page loads `@thatopen/components` (and its
+  `web-ifc` + Three.js peer dependencies) from a CDN via an ES module importmap.
+  It constructs the Forgejo raw file URL from the ifc:// URL parameters and
+  fetches the IFC bytes directly from Forgejo.
+
+### Current scope (Phase 3b)
+
+- Display the full model for whatever ref is shown in Forgejo.
+- Basic orbit/pan/zoom camera controls.
+- No filtering or selection UI yet.
+
+### Future scope
+
+- Element filtering and selection using `@thatopen/components` classifier and
+  highlighter APIs.
+- Camera state serialised to an `ifc://` URL so the user can copy a view link
+  (feeds into Phase 4 Bonsai "Copy view URL").
+- Diff highlighting driven by ifcmerge output (see Phase 3c).
+
+---
+
+## Phase 3c — ifcmerge integration
+
+`ifcmerge` is a git custom merge driver that resolves conflicts in `.ifc` files
+that git's built-in 3-way merge cannot handle.  Where two branches have both
+modified an IFC file (a fork), git would report an unresolvable conflict;
+ifcmerge performs a semantics-aware merge and produces a valid merged IFC file.
+
+### Git merge driver configuration
+
+Register ifcmerge as the git merge driver for `.ifc` files on the Forgejo
+server:
+
+```
+# /etc/gitconfig or ~forgejo/.gitconfig
+[merge "ifcmerge"]
+    name = IFC merge driver
+    driver = ifcmerge %O %A %B %L
+```
+
+```
+# .gitattributes in each repo, or server-side gitattributes
+*.ifc merge=ifcmerge
+```
+
+With this in place, Forgejo's existing "merge automatically" path will invoke
+ifcmerge via git when merging `.ifc` files, and the PR merge button will work
+without any Go-side changes for the common case.
+
+### Forgejo integration
+
+- Verify that Forgejo's merge-ability check (the "these branches can be merged
+  automatically" indicator) correctly reflects ifcmerge's ability to merge
+  `.ifc` files — investigate whether the check runs a trial merge via git or
+  uses a heuristic.
+- If needed, patch Forgejo to run a trial `git merge --no-commit` to get an
+  accurate result for IFC files.
+
+### 3D diff view (future)
+
+- After a merge completes, surface a "View merge in 3D" button in the PR using
+  the Phase 3b viewer.
+- Use `@thatopen/components` highlighter to colour added, removed, and modified
+  elements differently, driven by comparing the merged IFC against either parent.
+
+---
+
 ## Phase 4 — Bonsai integration
 
 Add `ifc://` handling to Bonsai.

ifcurl/service.py

@@ -39,6 +39,7 @@ import hashlib
 import os
 import tempfile
 import threading
+import time
 from collections import OrderedDict
 from pathlib import Path
 
@@ -128,11 +129,11 @@ def _guids_to_step_ids(model: ifcopenshell.file, guids: frozenset[str]) -> list[
 
 
 # ---------------------------------------------------------------------------
-# Tier 4: sha256(url) → PNG (filesystem, immutable refs only)
+# Tier 4: sha256(url) → PNG (filesystem, immutable refs only, no expiry)
 # ---------------------------------------------------------------------------
 
 def _t4_path(url: str) -> Path:
-    cache_dir = Path(user_cache_dir("ifcurl")) / "renders"
+    cache_dir = Path(user_cache_dir("ifcurl")) / "renders" / "immutable"
     cache_dir.mkdir(parents=True, exist_ok=True)
     url_hash = hashlib.sha256(url.encode()).hexdigest()
     return cache_dir / f"{url_hash}.png"
@@ -149,6 +150,36 @@ def _t4_put(url: str, png: bytes) -> None:
     _t4_path(url).write_bytes(png)
 
 
+# ---------------------------------------------------------------------------
+# Tier 4m: sha256(url) → PNG (filesystem, mutable refs, 5-minute TTL)
+# ---------------------------------------------------------------------------
+
+_T4M_TTL = 300  # seconds
+
+
+def _t4m_path(url: str) -> Path:
+    cache_dir = Path(user_cache_dir("ifcurl")) / "renders" / "mutable"
+    cache_dir.mkdir(parents=True, exist_ok=True)
+    url_hash = hashlib.sha256(url.encode()).hexdigest()
+    return cache_dir / f"{url_hash}.png"
+
+
+def _t4m_get(url: str) -> tuple[bytes, int] | None:
+    """Return (png_bytes, remaining_seconds) or None if missing/expired."""
+    path = _t4m_path(url)
+    try:
+        remaining = int(os.path.getmtime(path) + _T4M_TTL - time.time())
+        if remaining <= 0:
+            return None
+        return path.read_bytes(), remaining
+    except (FileNotFoundError, OSError):
+        return None
+
+
+def _t4m_put(url: str, png: bytes) -> None:
+    _t4m_path(url).write_bytes(png)
+
+
 # ---------------------------------------------------------------------------
 # Helper: load model from bytes via a temp file
 # ---------------------------------------------------------------------------
@@ -213,11 +244,24 @@ def preview(request: PreviewRequest) -> Response:
     if ifc_url.path is None:
         raise HTTPException(status_code=400, detail="URL has no 'path' parameter")
 
-    # --- Tier 4: cached PNG for immutable refs ---
-    if not ifc_url.is_mutable_ref():
+    # --- Tier 4 / 4m: cached PNG ---
+    if ifc_url.is_mutable_ref():
+        t4m_hit = _t4m_get(request.url)
+        if t4m_hit is not None:
+            cached_png, remaining = t4m_hit
+            return Response(
+                content=cached_png,
+                media_type="image/png",
+                headers={"Cache-Control": f"public, max-age={remaining}"},
+            )
+    else:
         cached_png = _t4_get(request.url)
         if cached_png is not None:
-            return Response(content=cached_png, media_type="image/png")
+            return Response(
+                content=cached_png,
+                media_type="image/png",
+                headers={"Cache-Control": "public, max-age=31536000, immutable"},
+            )
 
     # --- Resolve authentication token ---
     token = request.token
@@ -284,8 +328,18 @@ def preview(request: PreviewRequest) -> Response:
     except ValueError as exc:
         raise HTTPException(status_code=422, detail=str(exc)) from exc
 
-    # --- Tier 4: store PNG for immutable refs ---
-    if not ifc_url.is_mutable_ref():
+    # --- Tier 4 / 4m: store PNG and return with appropriate cache headers ---
+    if ifc_url.is_mutable_ref():
+        _t4m_put(request.url, png_bytes)
+        return Response(
+            content=png_bytes,
+            media_type="image/png",
+            headers={"Cache-Control": f"public, max-age={_T4M_TTL}"},
+        )
+    else:
         _t4_put(request.url, png_bytes)
-
-    return Response(content=png_bytes, media_type="image/png")
+        return Response(
+            content=png_bytes,
+            media_type="image/png",
+            headers={"Cache-Control": "public, max-age=31536000, immutable"},
+        )

tests/test_service.py

@@ -24,8 +24,9 @@ SELECTOR_URL = "ifc://example.com/org/repo@heads/main?path=model.ifc&selector=If
 
 
 @pytest.fixture(autouse=True)
-def clear_caches():
-    """Reset in-memory caches and tier-4 filesystem cache between tests."""
+def clear_caches(tmp_path, monkeypatch):
+    """Reset in-memory caches and redirect filesystem PNG cache between tests."""
+    monkeypatch.setattr("ifcurl.service.user_cache_dir", lambda *a, **kw: str(tmp_path))
     _t2_cache.clear()
     _t3_cache.clear()
     yield
@@ -34,9 +35,8 @@ def clear_caches():
 
 
 @pytest.fixture()
-def tmp_t4_cache(tmp_path, monkeypatch):
-    """Redirect tier-4 PNG cache to a temp directory."""
-    monkeypatch.setattr("ifcurl.service.user_cache_dir", lambda *a, **kw: str(tmp_path))
+def tmp_t4_cache(tmp_path):
+    """Return the temp cache directory (already redirected by clear_caches)."""
     return tmp_path
 
 
@@ -109,7 +109,7 @@ class TestTier2Cache:
             client.post("/preview", json={"url": MUTABLE_URL})
         assert (FAKE_HEXSHA, "model.ifc") in _t2_cache
 
-    def test_second_request_uses_cached_bytes(self, model_with_geometry):
+    def test_second_request_uses_cached_bytes(self, model_with_geometry, monkeypatch):
         ifc_bytes = model_with_geometry.to_string().encode()
         call_count = 0
 
@@ -118,6 +118,9 @@ class TestTier2Cache:
             call_count += 1
             return FAKE_HEXSHA, ifc_bytes
 
+        # Disable tier-4m so both requests reach fetch_ifc, letting us
+        # verify that tier-2 serves ifc_bytes from cache on the second call.
+        monkeypatch.setattr("ifcurl.service._T4M_TTL", 0)
         with patch("ifcurl.service.fetch_ifc", counting_fetch), \
              patch("ifcurl.service.render_mod.render", return_value=FAKE_PNG):
             client.post("/preview", json={"url": MUTABLE_URL})
@@ -192,7 +195,7 @@ class TestTier4Cache:
         assert r2.content == FAKE_PNG
         assert render_call_count == 1  # render only called once
 
-    def test_png_not_cached_for_mutable_ref(self, tmp_t4_cache, model_with_geometry):
+    def test_png_cached_for_mutable_ref_within_ttl(self, tmp_t4_cache, model_with_geometry):
         ifc_bytes = model_with_geometry.to_string().encode()
         render_call_count = 0
 
@@ -206,7 +209,24 @@ class TestTier4Cache:
             client.post("/preview", json={"url": MUTABLE_URL})
             client.post("/preview", json={"url": MUTABLE_URL})
 
-        assert render_call_count == 2  # no tier-4 cache for mutable ref
+        assert render_call_count == 1  # second request served from tier-4m cache
+
+    def test_png_not_cached_for_mutable_ref_after_ttl(self, tmp_t4_cache, model_with_geometry, monkeypatch):
+        ifc_bytes = model_with_geometry.to_string().encode()
+        render_call_count = 0
+
+        def counting_render(*args, **kwargs):
+            nonlocal render_call_count
+            render_call_count += 1
+            return FAKE_PNG
+
+        monkeypatch.setattr("ifcurl.service._T4M_TTL", 0)
+        with patch("ifcurl.service.fetch_ifc", _mock_fetch(ifc_bytes)), \
+             patch("ifcurl.service.render_mod.render", counting_render):
+            client.post("/preview", json={"url": MUTABLE_URL})
+            client.post("/preview", json={"url": MUTABLE_URL})
+
+        assert render_call_count == 2  # TTL=0 means cache always expired
 
     def test_tier4_cache_file_exists_on_disk(self, tmp_t4_cache, model_with_geometry):
         ifc_bytes = model_with_geometry.to_string().encode()