Skip to content

fix(mobile): hide placeholder session titles and disambiguate duplicate artifact names - #6759

Merged
iscekic merged 2 commits into
mainfrom
kwf/janitor-mobile-artifacts-1559370511
Sep 28, 2026
Merged

iscekic merged 2 commits into
mainfrom
kwf/janitor-mobile-artifacts-1559370511

Conversation

@iscekic

@iscekic iscekic commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Fix proof

A session that still carries the backend's placeholder title (New session - <ISO> / Child session - <ISO>) is shown in the phone's file browser as that machine string, because `buildSessionArtifac

Asserted value: apps/mobile/src/lib/artifacts/artifact-crawl.ts. Sense check (jev): probability 0.93

The scripts were proven on an earlier base, so only the head ran.

Head 7da128024d17

Head log: backend-assert 12df6407ebc6 exited 0
$ git diff --unified=0 d72f1f366dabae2752f480d1c65dff586f12d90f 7da128024d17a2b1e04c5cb3f644f8a7d9557dca -- apps/mobile/src/lib/artifacts/artifact-crawl.ts
diff --git a/apps/mobile/src/lib/artifacts/artifact-crawl.ts b/apps/mobile/src/lib/artifacts/artifact-crawl.ts
--- a/apps/mobile/src/lib/artifacts/artifact-crawl.ts
+++ b/apps/mobile/src/lib/artifacts/artifact-crawl.ts
+      id: session.id,
+      title: sessionDisplayTitle(session.title) ?? null,
+    }),
@@ -347 +357,3 @@ export function buildSessionArtifacts(
-    files: (artifactsBySession.get(session.id) ?? []).map(artifact => toMirrorFile(artifact)),
+    files: uniqueArtifactDisplayNames(
+      (artifactsBySession.get(session.id) ?? []).map(artifact => toMirrorFile(artifact))
+    ),

Two artifacts in one session that share a filename are written to the manifest with the same display name, so the OS file browser shows duplicate, indistinguishable entries in the session folder; `b

Asserted value: apps/mobile/src/lib/artifacts/artifact-crawl.ts. Sense check (jev): probability 0.92

The scripts were proven on an earlier base, so only the head ran.

Head 7da128024d17

Head log: backend-assert 12df6407ebc6 exited 0
$ git diff --unified=0 d72f1f366dabae2752f480d1c65dff586f12d90f 7da128024d17a2b1e04c5cb3f644f8a7d9557dca -- apps/mobile/src/lib/artifacts/artifact-crawl.ts
diff --git a/apps/mobile/src/lib/artifacts/artifact-crawl.ts b/apps/mobile/src/lib/artifacts/artifact-crawl.ts
--- a/apps/mobile/src/lib/artifacts/artifact-crawl.ts
+++ b/apps/mobile/src/lib/artifacts/artifact-crawl.ts
+      id: session.id,
+      title: sessionDisplayTitle(session.title) ?? null,
+    }),
@@ -347 +357,3 @@ export function buildSessionArtifacts(
-    files: (artifactsBySession.get(session.id) ?? []).map(artifact => toMirrorFile(artifact)),
+    files: uniqueArtifactDisplayNames(
+      (artifactsBySession.get(session.id) ?? []).map(artifact => toMirrorFile(artifact))
+    ),

Changelog for users

  • The phone's file browser shows Session <id> instead of a raw New session - <ISO> folder for sessions that still carry the backend placeholder.
  • Two artifacts sharing a filename in one session appear as distinct entries, with later ones suffixed (2), (3), and so on.

Changelog for maintainers

  • The crawl now gates each raw title through sessionDisplayTitle before sanitizing, so a placeholder row reaches the manifest as a null title and falls back to Session <id>.
  • New uniqueArtifactDisplayNames suffixes duplicates before the extension, keeps every name inside the byte bound, and returns the input files unmutated.
  • The artifact file-provider contract test now asserts the gated call shape; changing that call fails the contract.
  • Review the extension split first: a name without a dot takes a bare (n) suffix, and the extension is capped so the counter keeps room.

E2E proof

A session that still carries the backend's placeholder title (New session - <ISO> / Child session - <ISO>) is shown in the phone's file browser as that machine string, because buildSessionArtifacts passes the raw session.title straight to safeArtifactSessionName instead of gating it through the shared sessionDisplayTitle that every app surface uses to hide the placeholder; the user opening the Kilo location sees a raw timestamp folder name the app itself would never paint.

Code trace: apps/mobile/src/lib/artifacts/artifact-crawl.ts:12 changed in c59cdf4a9d979f20addf729e7ae1ce07eec439e4. Sense check (jev): probability 0.92

Changed lines
+  uniqueArtifactDisplayNames,
+import { sessionDisplayTitle } from '@/lib/session-display-title';
- * non-empty path component, with `Session <id>` as the fallback.
+ * non-empty path component, with `Session <id>` as the fallback. The title is
+ * first gated through {@link sessionDisplayTitle}, so a row still carrying the
+ * backend's `New session - <ISO>` placeholder gets the fallback label instead
+ * of the machine string the app itself would never paint. Duplicate filenames
+ * within a session are disambiguated by {@link uniqueArtifactDisplayNames}, so
+ * the browser shows one row per artifact.
-    title: safeArtifactSessionName({ id: session.id, title: session.title }),
+    title: safeArtifactSessionName({
+      id: session.id,
+      title: sessionDisplayTitle(session.title) ?? null,
+    }),
-    files: (artifactsBySession.get(session.id) ?? []).map(artifact => toMirrorFile(artifact)),
+    files: uniqueArtifactDisplayNames(
+      (artifactsBySession.get(session.id) ?? []).map(artifact => toMirrorFile(artifact))
+    ),

Two artifacts in one session that share a filename are written to the manifest with the same display name, so the OS file browser shows duplicate, indistinguishable entries in the session folder; buildSessionArtifacts maps each file through toMirrorFile independently and never de-duplicates names within the session.

Code trace: apps/mobile/src/lib/artifacts/artifact-crawl.ts:12 changed in c59cdf4a9d979f20addf729e7ae1ce07eec439e4. Sense check (jev): probability 0.9

Changed lines
+  uniqueArtifactDisplayNames,
+import { sessionDisplayTitle } from '@/lib/session-display-title';
- * non-empty path component, with `Session <id>` as the fallback.
+ * non-empty path component, with `Session <id>` as the fallback. The title is
+ * first gated through {@link sessionDisplayTitle}, so a row still carrying the
+ * backend's `New session - <ISO>` placeholder gets the fallback label instead
+ * of the machine string the app itself would never paint. Duplicate filenames
+ * within a session are disambiguated by {@link uniqueArtifactDisplayNames}, so
+ * the browser shows one row per artifact.
-    title: safeArtifactSessionName({ id: session.id, title: session.title }),
+    title: safeArtifactSessionName({
+      id: session.id,
+      title: sessionDisplayTitle(session.title) ?? null,
+    }),
-    files: (artifactsBySession.get(session.id) ?? []).map(artifact => toMirrorFile(artifact)),
+    files: uniqueArtifactDisplayNames(
+      (artifactsBySession.get(session.id) ?? []).map(artifact => toMirrorFile(artifact))
+    ),
Owner request

Fix 2 janitor findings in mobile/artifacts. Fix every one; the proof covers each.

  1. A session that still carries the backend's placeholder title (New session - <ISO> / Child session - <ISO>) is shown in the phone's file browser as that machine string, because buildSessionArtifacts passes the raw session.title straight to safeArtifactSessionName instead of gating it through the shared sessionDisplayTitle that every app surface uses to hide the placeholder; the user opening the Kilo location sees a raw timestamp folder name the app itself would never paint.
    Trace: apps/mobile/src/lib/artifacts/artifact-crawl.ts:345: A session that still carries the backend's placeholder title (New session - <ISO> / Child session - <ISO>) is shown in the phone's file browser as that machine string, because buildSessionArtifacts passes the raw session.title straight to safeArtifactSessionName instead of gating it through the shared sessionDisplayTitle that every app surface uses to hide the placeholder; the user opening the Kilo location sees a raw timestamp folder name the app itself would never paint. (janitor area experience-design).
    Files: apps/mobile/src/lib/artifacts/artifact-crawl.ts.
  2. Two artifacts in one session that share a filename are written to the manifest with the same display name, so the OS file browser shows duplicate, indistinguishable entries in the session folder; buildSessionArtifacts maps each file through toMirrorFile independently and never de-duplicates names within the session.
    Trace: apps/mobile/src/lib/artifacts/artifact-crawl.ts:347: Two artifacts in one session that share a filename are written to the manifest with the same display name, so the OS file browser shows duplicate, indistinguishable entries in the session folder; buildSessionArtifacts maps each file through toMirrorFile independently and never de-duplicates names within the session. (janitor area experience-design).
    Files: apps/mobile/src/lib/artifacts/artifact-crawl.ts.

@iscekic iscekic added the kwf-janitor Admitted to the workflow from a janitor finding label Sep 26, 2026
@kilo-code-bot

kilo-code-bot Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit 7da128024d17 ("satisfy artifact mirror lint rules") is a behavior-preserving refactor of uniqueArtifactDisplayName (now taking the ArtifactMirrorFile object), a new ArtifactDisplayNameParts return type for splitArtifactExtension, and a relocation of the uniqueArtifactDisplayNames tests into artifact-mirror-manifest-names.test.ts; no new issues and no memory-leak surface.

Files Reviewed (3 incremental files)
  • apps/mobile/src/lib/artifacts/artifact-mirror-manifest.ts
  • apps/mobile/src/lib/artifacts/artifact-mirror-manifest-names.test.ts
  • apps/mobile/src/lib/artifacts/artifact-mirror-manifest.test.ts
Previous Review Summary (commit 3bdc536)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 3bdc536)

Status: No Issues Found | Recommendation: Merge

Reviewed the mobile artifact mirror changes: placeholder session titles are now gated through sessionDisplayTitle before safeArtifactSessionName, and duplicate display names within a session are suffixed before the extension. Verified the suffix loop always finds a free candidate (used.size + 1 distinct candidates against at most used.size taken), that every suffixed name stays within the 200-byte bound with its extension intact, and that the input files are never mutated. The new tests and the updated file-provider contract assertion align with the implementation, and existing buildSessionArtifacts session-label tests remain compatible. No memory-leak surface was introduced (pure functions with a local Set).

Files Reviewed (5 files)
  • apps/mobile/src/lib/artifacts/artifact-crawl.ts
  • apps/mobile/src/lib/artifacts/artifact-crawl.test.ts
  • apps/mobile/src/lib/artifacts/artifact-mirror-manifest.ts
  • apps/mobile/src/lib/artifacts/artifact-mirror-manifest.test.ts
  • apps/mobile/src/lib/artifacts/artifacts-file-provider-contract.test.ts

Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

- extract the unique-name tests to their own file to stay under max-lines
- pass the artifact to uniqueArtifactDisplayName to fit max-params
- name the splitArtifactExtension return type so the anti-slop rule accepts it
@iscekic

iscekic commented Sep 26, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Fixed failing checks in 7da1280.

@iscekic iscekic self-assigned this Sep 26, 2026
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 26, 2026
@iscekic iscekic added merge-by-human the merge bot routed this PR to a human merge-by-bot and removed human-ready The PR is ready for human review. merge-by-human the merge bot routed this PR to a human labels Sep 26, 2026
@iscekic
iscekic marked this pull request as draft September 28, 2026 08:40
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 08:44
@iscekic
iscekic marked this pull request as draft September 28, 2026 08:53
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:01
@iscekic
iscekic marked this pull request as draft September 28, 2026 09:05
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:10
@iscekic
iscekic marked this pull request as draft September 28, 2026 09:17
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kwf-janitor Admitted to the workflow from a janitor finding merge-by-bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants