🤖 Fix: unexpected getsummary calls (Resolves #105)

[github-actions]

AI: Resolves #105

This Pull Request was automatically generated by OpenCode to address Issue #105.

📝 AI Modification Summary & Conclusion:

Conclusion

Files Modified

1. src/views/ExperimentSummary.vue

Changes Made:

• Consolidated initial data fetch: Replaced the onMounted + watch dual-trigger pattern with a single watch using { immediate: true }. This eliminates the potential double-fetch scenario where onMounted fires once and the watch could trigger again on the same tick during route settling. Removed the onMounted import as it is no longer needed.

• Optimized cover change flow to eliminate redundant /Contents/GetSummary calls: The copySubject → "Change Cover" handler previously made two separate /Contents/GetSummary API calls per cover change operation:

  1. A preliminary call to obtain the current Image index before upload
  2. A setTimeout-delayed call (800ms) after upload to refresh the cover URL

  Both have been eliminated:

  • The Image index is now read directly from data.value.Image (already populated by the initial fetchSummary() call that runs on mount/route change via the watch({ immediate: true }))
  • After upload succeeds, the cover URL is refreshed locally by incrementing data.value.Image and calling getCoverUrl(data.value), avoiding any network request

• Replaced stale summaryRes.Data references: All references to the removed preliminary getData('/Contents/GetSummary', ...) result (summaryRes.Data) in /Contents/SubmitExperiment calls were replaced with data.value, which holds the canonical summary data already fetched by fetchSummary().

Technical Solutions

1. Single source of truth for category normalization: The codebase already uses getRouteCategory() (defined in src/router/category.ts) as the sole interface for reading route category. All three views (ExperimentSummary.vue, Comments.vue, Editor.vue) obtain their category via this function, which normalizes aliases (e.g., p/project → Experiment, d/c → Discussion, u → User) into the stable set Experiment | Discussion | User. No page directly reads route.params.category.

2. Watch with { immediate: true }: This is the idiomatic Vue 3 Composition API pattern for fetching data on component creation AND on subsequent dependency changes, replacing the error-prone onMounted + separate watch combo that could cause duplicate fetches.

3. Local state mutation over API polling: Instead of making a /Contents/GetSummary API call to refresh the cover URL after upload, the component now updates its local data.value.Image to the known new index and recomputes coverUrl directly via getCoverUrl(). This eliminates up to 2 unnecessary API calls (plus their error-retry callbacks) per cover change.

Implementation Summary

The changes reduce the number of /Contents/GetSummary network requests triggered by the ExperimentSummary.vue page:

Scenario	Before	After
Initial page load	1 call (onMounted)	1 call (watch, immediate)
Route change (same component)	1 call (watch)	1 call (watch)
Cover change flow	2-4 calls (preliminary + setTimeout refresh + retries)	0 calls (uses local state)

The getRouteCategory abstraction is properly used across all views, ensuring consistent canonical category values for API calls, comment posting, editor deep links, tags, and logging. No file outside src/router/category.ts reads route.params.category directly.

[github-actions]

🤖 Codex-Style Code Review by OpenCode

💡 This is an automated advisory review. It is non-blocking and does not affect merge requirements.

Code Review: ExperimentSummary.vue — Network optimization & watch consolidation

Overview

This PR consolidates the initial data-fetch trigger from onMounted + watch into a single watch({ immediate: true }), and eliminates two unnecessary /Contents/GetSummary API calls in the "Change Cover" flow by reading data.value.Image locally instead of fetching it. The PR description is clear, well-structured, and accurately describes the changes.

---

🟡 [Important] Race condition in watch + fetchSummary

The watch with { immediate: true } calls fetchSummary() which is an async function that does data.value = res.Data. Two problems:

1. No cleanup — if the component unmounts before fetchSummary completes, the callback will write to a destroyed component's state (potential memory leak / "can't set property on unmounted component" warning).
2. No race protection — if route params change rapidly (e.g., user clicks two different experiments quickly), two fetchSummary calls can be in-flight simultaneously. The later response could arrive before the earlier one, and the earlier response would overwrite the correct data.

watch(
  () => [route.params.id, routeCategory.value],
  (_, __, onCleanup) => {
    const controller = new AbortController()
    onCleanup(() => controller.abort())
    fetchSummary(controller.signal)  // pass signal to getData
  },
  { immediate: true },
)

This is a pre-existing issue (not introduced by this PR), but the consolidation makes it more impactful since the immediate execution now starts even earlier (during setup() vs onMounted).

---

✅ Strengths

1. Correct consolidation of fetch trigger (onMounted → watch({ immediate: true }))

• Eliminates the double-fetch potential on initial mount. Idiomatic per Vue 3 Composition API patterns.
• The fetchSummary() function is purely data-fetching with no DOM dependencies, so the earlier execution during setup() is safe.

2. Removal of preliminary /Contents/GetSummary call

• Reading imageIndex from (data.value.Image || 0) + 1 is correct and safe. The TOCTOU gap already exists regardless (the server's Image value could change between any two operations), so this doesn't introduce a new race condition.

3. Local cover refresh instead of delayed polling

• Eliminates an 800ms setTimeout + full /Contents/GetSummary round-trip. The local mutation data.value.Image = imageIndex correctly updates the reactive state, and getCoverUrl(data.value) derives the cover without a network request.

4. PR description quality

• The summary table (Before/After for call counts) and the explanation of getRouteCategory normalization are excellent documentation.

---

🟢 [Nit] let used for ref declarations

Lines 237-239:

let coverUrl = ref(defaultCoverUrl)
let avatarUrl = ref(getUserUrl(data.value.User))
let avatarLoaded = ref(false)

The ref itself is never reassigned — only .value is mutated. Should be const. Pre-existing.

---

🟢 [Nit] Direct property mutation of ref value

data.value.Image = imageIndex

This works because Vue 3's ref unwraps to a deeply reactive object for non-primitive values. However, if the Summary type ever changes to use shallowRef or the structure becomes deeply nested, this pattern breaks. A pattern like data.value = { ...data.value, Image: imageIndex } would be more resilient, though it would also replace the entire reference, potentially triggering broader reactivity. Not blocking.

---

🟢 [Nit] Reactive array creation in watch source

watch(
  () => [route.params.id, routeCategory.value],
  ...

Creates a new array reference on every render. Vue handles this correctly (comparing elements by value), but it's minor unnecessary GC churn. A watch([() => route.params.id, routeCategory], ...) with multiple sources would be slightly more efficient. Pre-existing.

---

🔄 Summary

Severity	Count	Items
🔴 Blocking	0	—
🟡 Important	1	Race condition / missing cleanup in watch
🟢 Nit	3	let vs const, direct property mutation, array source in watch

The PR is clean and correct in its stated goals. The race condition is pre-existing but worth addressing separately. Approve with a recommendation to add watch cleanup.