feat: report remodel recipe v3 facts

[KagamiChan]

Summary

• emit v3 remodel recipe availability, detail cost/material, and conversion update facts
• keep legacy v2 remodel recipe reporting intact
• bundle reporters into the single plugin output instead of shipping generated reporter JS files
• inject reporter version at build time so package.json is not bundled into index.js
• add an opt-in, in-memory remodel debug recorder exposed through a functional React/TSX plugin settings panel
• export only allowlisted, sanitized capture data when the user clicks Export
• add Storybook preview stories for the settings component
• add reporter and recorder regression coverage for no-helper context, malformed requirements, stale state, conversion targets, bounded sanitized capture, subscriptions, and deterministic timestamps

Testing

• pnpm run typecheck
• pnpm run lint
• pnpm run storybook:build
• pnpm run build
• pnpm run smoke
• pnpm run test:coverage

[Copilot]

Pull request overview

This PR extends RemodelRecipeReporter to emit v3 item improvement “facts” (availability/list, detail/cost+materials, and execution/update) while keeping legacy v2 execution reporting, and adds regression tests for edge cases (missing fleet context, malformed requirements, stale/mismatched state, conversion targets).

Changes:

• Add v3 reporting payloads for list availability, detail costs/material requirements, and execution conversion updates.
• Remove moment-timezone usage and switch day calculation to a Date-based JST day helper, plus stronger validation/normalization around detail data.
• Expand reporter tests to cover v3 reporting, malformed requirement pairs, missing fleet context, and mismatch/stale-state scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
tests/reporters/remodel-recipe.test.ts	Adds/updates regression tests validating v3 facts emission and edge-case handling.
src/reporters/remodel-recipe.ts	Implements v3 reporting, fleet context extraction, required-item normalization, and Date-based JST day computation; preserves v2 execution report.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

My review of the PR follows.

---

Finding 1 (Medium — Flaky test dependent on wall-clock time)

The test "reports successful remodel recipes with cached recipe cost and day data" (lines 110–168 of the test file) calls handle() without an explicit time argument in three places, which defaults to Date.now(). The test then asserts exact day: 6 values for both the v2 and v3 reports:

// lines 115-119 — no time arg
reporter.handle('GET', '/kcsapi/api_req_kousyou/remodel_slotlist', listBody)
reporter.handle('POST', '/kcsapi/api_req_kousyou/remodel_slotlist_detail', detailBody, {
  api_id: '33',
  api_slot_id: 501,
})
// lines 120-132 — no time arg
reporter.handle('POST', '/kcsapi/api_req_kousyou/remodel_slot', ...)

getJstDay(Date.now()) returns the current real-world JST day-of-week. The test hardcodes day: 6 (Saturday). It only passes when the test runs on a JST Saturday; on any other day the assertion fails.

Fix: Pass an explicit timestamp to each handle() call, e.g. Date.UTC(2026, 6, 4, 0) (Saturday in JST at 00:00 UTC = 09:00 JST), so the behavior is deterministic.

---

Finding 2 (Low — clientObservedAt in execution payload uses stale detail timestamp)

In the execution handler at src/reporters/remodel-recipe.ts:606:

clientObservedAt: currentDetail.clientObservedAt,

This sets the execution-event timestamp to the time the detail was observed (possibly much earlier), rather than the time the execution itself occurred. The handle() method already receives the current time parameter for the execution call. The availability and cost payloads both use the time parameter directly; the update payload is the only one that diverges.

This produces inaccurate timing data for v3 update records. If a user loads the detail screen, waits 10 minutes, then executes the remodel, the execution record will be stamped 10 minutes in the past.

Fix: Use the time parameter from the execution handler's arguments:

clientObservedAt: time,

The fix is a one-character-level change (replacing currentDetail.clientObservedAt with time), assuming time is added to destructured usage or referenced directly.

opencode session  |  github run

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

[github-actions]

I've now reviewed all 6 changed files. Let me confirm my understanding of the codebase.

Findings already identified by existing reviews:

1. Finding 1 (Medium — Flaky test): The test "reports successful remodel recipes with cached recipe cost and day data" (test file lines 115-119) calls handle() without an explicit time argument in two places, defaulting to Date.now(). The assertions hardcode day: 6 and key: 'r33-i700-s1-d6-s99', which only pass when getJstDay(Date.now()) returns 6 — roughly 1/7 of the time. Confirmed valid.

2. Finding 2 (Low — Stale clientObservedAt): At src/reporters/remodel-recipe.ts:606, the v3 execution payload uses currentDetail.clientObservedAt (the detail observation time) instead of time (the execution call time). Confirmed valid.

---

Additional finding not covered by existing reviews:

Finding 3 (Low — Stale day in v3 execution payload)

At src/reporters/remodel-recipe.ts:610, the v3 execution payload uses day: currentDetail.day, which was computed from the detail observation timestamp via getJstDay(detailTime). If the user views the detail screen near JST midnight and then executes the remodel after midnight, the day-of-week in the execution record will be wrong by one.

This is the same root cause as Finding 2 (stale detail data leaking into the execution payload), but requires a different fix — getJstDay(time) rather than just time. The time parameter is already available in scope at line 530 (time = Date.now() on the handle signature).

Fix (line 610):

- day: currentDetail.day,
+ day: getJstDay(time),

Note: Finding 2 and Finding 3 should be fixed together since they share the same stale-data pattern in the same payload block (lines 603-617). All time-derived fields in the execution payload should use the execution-call time parameter rather than currentDetail.

opencode session  |  github run

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[github-actions]

I've reviewed all 6 changed source files thoroughly. After cross-referencing with the existing review comments, I find no material issues beyond those already identified. Here's my independent assessment:

Already-covered findings I've verified:

1. Flaky test (test file line 115-119) — The test hardcodes day: 6 but relies on Date.now() via defaulted time parameters. Will fail ~6/7 of the time. Legitimate.

2. Stale clientObservedAt (src/reporters/remodel-recipe.ts:606) — Execution payload uses currentDetail.clientObservedAt (detail observation time) instead of the time parameter already in scope. Legitimate.

3. Stale day (src/reporters/remodel-recipe.ts:610) — Execution payload uses currentDetail.day instead of getJstDay(time). Same root cause as Add support of api v2 #2. Legitimate.

4. sanitizeSlotBody drops boolean api_remodel_flag (src/remodel-debug-recorder.ts:150) — pickNumber rejects booleans, silently dropping success/failure signal from debug exports. Legitimate but debug-only.

5. Potential TypeError in createSanitizedContext (src/remodel-debug-recorder.ts:177) — window._decks[0]?.api_ship.slice(0, 2) calls .slice on undefined when api_ship is missing. Contained by the recorder's internal try-catch, so it won't crash production reporting. Legitimate but debug-only.

I found no additional vulnerabilities or correctness issues beyond these five.

opencode session  |  github run

[Copilot]

Pull request overview

Copilot reviewed 14 out of 16 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[github-actions]

Reviewed all 16 changed files against the PR diff and cross-verified every finding from 6 existing review comments — all were based on outdated or misread code (tests pass explicit timestamps, execution payload already uses time/getJstDay(time), sanitizer correctly preserves booleans, optional chaining guards are in place). Found no material issues to report.

opencode session  |  github run

[github-actions]

After a thorough analysis of all 16 changed files and the full diff, I concur with the final review comment (06:34:01Z). The earlier findings (flaky tests, stale timestamps, dropped booleans, optional chaining crashes) are all based on misread code:

• Tests use vi.useFakeTimers() + vi.setSystemTime() in beforeEach, and all day-dependent assertions have matching explicit timestamps. Deterministic.
• v3 execution payload already uses time for clientObservedAt (line 606) and getJstDay(time) for day (line 610) — not the cached detail values.
• sanitizeSlotBody uses pickBooleanOrNumber (line 177), which handles both boolean and number.
• createSanitizedContext uses api_ship?.slice(0, 2) (line 204) — the ?. guards api_ship before .slice.
• setRemodelDebugRecorderEnabled is wrapped in try-catch (lines 61-69).
• React is in onlyBundle, not neverBundle — it gets bundled into the output.

No material correctness, security, or reliability issues to report.

opencode session  |  github run

[Copilot]

Pull request overview

Copilot reviewed 14 out of 16 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[Copilot]

Pull request overview

Copilot reviewed 14 out of 17 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[Copilot]

Pull request overview

Copilot reviewed 22 out of 26 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[github-actions]

No material issues to report.

After thorough review of all 26 changed files and cross-referencing every claim from the 16+ existing review comments against the actual source code, all previously reported findings are based on misreadings:

• Tests (beforeEach at remodel-recipe.test.ts:11-14): Use vi.useFakeTimers() + vi.setSystemTime(fixedTestTime) — all Date.now() calls are deterministic. Day-dependent assertions pass explicit time arguments. Verified all JST day calculations match expectations.
• v3 execution clientObservedAt (remodel-recipe.ts:608): Uses time from the handle() parameter, not currentDetail.clientObservedAt.
• v3 execution day (remodel-recipe.ts:612): Uses getJstDay(time), not currentDetail.day.
• sanitizeSlotBody (remodel-debug-recorder.ts:189): Uses pickBooleanOrNumber() which handles booleans.
• createSanitizedContext (remodel-debug-recorder.ts:217): Uses window._decks?.[0]?.api_ship?.slice(0, 2) || [] — fully guarded with array fallback.
• setRemodelDebugRecorderEnabled (remodel-debug-recorder.ts:71-83): try-catch wrapped; in-memory cache updated before storage.
• localStorage hot path (remodel-debug-recorder.ts:285): Path check runs before isRemodelDebugRecorderEnabled() (:288), which uses enabledCache in-memory.
• Records: Capped at 200 (MAX_RECORDS); getRemodelDebugRecords returns deep-cloned snapshots via JSON.parse(JSON.stringify(...)).
• useEffect unsubscribe (remodel-debug-settings.tsx:33-35): Returns cleanup function correctly.
• Storybook: Uses decorators (withRecorderState), not unsupported beforeEach.
• React bundling: neverBundle for React runtime modules is consistent with Poi plugin conventions; smoke loader stubs all of them.
• this.recipes set before context check (remodel-recipe.ts:384): This is intentional — it ensures v2 reporting continues to work even when v3 fleet context is unavailable, as validated by the test at line 436.

The v3 reporting logic, debug recorder sanitization, fleet context extraction, required-item normalization, and test coverage are all sound and well-tested. No correctness, security, performance, or reliability issues found.

opencode session  |  github run

[github-actions]

After a thorough review of all 26 changed files and independent verification of every finding from the existing review comments against the actual source code, I find no material issues to report.

Every previously claimed finding is based on misreadings of the code. Key verifications:

• Tests — beforeEach sets vi.useFakeTimers() + vi.setSystemTime(fixedTestTime) (remodel-recipe.test.ts:11-14). All Date.now() calls are deterministic. Day-dependent assertions use explicit time arguments.
• v3 execution clientObservedAt/day — remodel-recipe.ts:608,612 already uses time and getJstDay(time) from the execution handler's parameter, not stale cached values.
• sanitizeSlotBody — remodel-debug-recorder.ts:189 uses pickBooleanOrNumber(), which handles booleans. Verified by tests (remodel-debug-recorder.test.ts:160-188).
• createSanitizedContext — remodel-debug-recorder.ts:217 uses window._decks?.[0]?.api_ship?.slice(0, 2) || [] — fully guarded by optional chaining with array fallback. Tested at lines 247-265.
• setRemodelDebugRecorderEnabled — Lines 71-83 wrap localStorage in try-catch; in-memory enabledCache is updated before touching storage. Tested at lines 343-362.
• localStorage hot path — recordRemodelDebugEvent at line 285 checks REMODEL_PATHS.has(event.path) before isRemodelDebugRecorderEnabled() at line 288, which uses an in-memory cache. Tested at lines 70-128.
• Record capping — Enforced at 200 (MAX_RECORDS, lines 300-301). getRemodelDebugRecords returns deep-cloned copies via JSON.parse(JSON.stringify(...)). Both tested.
• useEffect unsubscribe — remodel-debug-settings.tsx:34 returns the unsubscribe function.
• notifyListeners wraps each listener invocation in try-catch (remodel-debug-recorder.ts:42-47).
• Storybook uses decorators, not beforeEach. React externals in neverBundle are consistent with Poi plugin conventions and stubbed in the smoke loader.

opencode session  |  github run

[Copilot]

Pull request overview

Copilot reviewed 22 out of 26 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[github-actions]

After a thorough independent review of all changed source files against the actual code (not the review comments), I find no material issues to report.

Every finding in the existing review comments is based on misreadings. Key verifications against the actual source:

• Tests: beforeEach sets vi.useFakeTimers() + vi.setSystemTime(fixedTestTime) (test file lines 11-14), making all Date.now() calls deterministic. Day-dependent assertions pass explicit time arguments.
• v3 execution clientObservedAt: remodel-recipe.ts:608 uses time from the handle() parameter, not cached detail data.
• v3 execution day: remodel-recipe.ts:612 uses getJstDay(time), not currentDetail.day.
• sanitizeSlotBody: Line 189 uses pickBooleanOrNumber(), which handles booleans. Verified by tests at lines 160-188.
• createSanitizedContext: Line 217 uses window._decks?.[0]?.api_ship?.slice(0, 2) || [] — fully guarded with array fallback. Tested at lines 247-264.
• setRemodelDebugRecorderEnabled: try-catch wrapped (lines 71-83); in-memory cache updated before storage. Tested at lines 343-362.
• localStorage hot path: REMODEL_PATHS.has(event.path) check at line 285 runs before isRemodelDebugRecorderEnabled() at line 288, which uses in-memory enabledCache. Tested at lines 70-128.
• Records: Capped at 200 (MAX_RECORDS, line 300-301); getRemodelDebugRecords returns deep-cloned copies via JSON.parse(JSON.stringify(...)). Both tested.
• useEffect unsubscribe: Returns cleanup function (line 34-35).
• Storybook stories: Use decorators (withRecorderState), not beforeEach.
• React bundling: neverBundle for React runtime modules is consistent with Poi plugin conventions; smoke loader stubs all of them.
• getReporterVersion: Vendor declaration at vendor.d.ts:23 declares __REPORTER_VERSION__: string | undefined. The dual-check (compile-time define first, then globalThis fallback) handles both bundled and test execution correctly.
• itemId change to ??: Line 436 — 0 is still rejected by the <= 0 guard at line 440.

The v3 reporting logic, debug recorder sanitization, fleet context extraction, required-item normalization, and test coverage are all sound.

opencode session  |  github run

[Copilot]

Pull request overview

Copilot reviewed 22 out of 26 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[Copilot]

Pull request overview

Copilot reviewed 22 out of 26 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[github-actions]

After exhaustive analysis of all 26 changed files and cross-referencing every claim from the multiple existing review comments against the actual source code, I find no material correctness, security, reliability, or maintenance issues to report.

Every finding in the existing review comments is based on misreadings of the code. Key verifications against the actual source:

• Tests (remodel-recipe.test.ts:11-14): beforeEach sets vi.useFakeTimers() + vi.setSystemTime(fixedTestTime). All Date.now() calls are deterministic. Every day-dependent assertion uses explicit time arguments or derives from the mocked system time. Not flaky.
• v3 execution clientObservedAt/day (remodel-recipe.ts:608,612): Already uses time and getJstDay(time) from the execution handler's parameter. No stale detail data.
• sanitizeSlotBody (remodel-debug-recorder.ts:189): Uses pickBooleanOrNumber(), which handles both booleans and numbers. Confirmed by test at remodel-debug-recorder.test.ts:160-188.
• createSanitizedContext (remodel-debug-recorder.ts:217): Uses window._decks?.[0]?.api_ship?.slice(0, 2) || [] — fully guarded by optional chaining with array fallback. Tested at lines 247-265 with both missing api_ship and missing _decks.
• localStorage hot path (remodel-debug-recorder.ts:285-289): REMODEL_PATHS.has(event.path) check runs before isRemodelDebugRecorderEnabled(), which uses in-memory enabledCache. Tested at lines 70-87.
• Records: Capped at 200 (MAX_RECORDS, line 300-301). getRemodelDebugRecords returns deep-cloned copies via JSON.parse(JSON.stringify(...)). Both tested.
• notifyListeners: Each listener invocation is wrapped in try-catch (remodel-debug-recorder.ts:42-47). Tested at lines 326-341.
• setRemodelDebugRecorderEnabled: try-catch wrapped; in-memory cache updated before touching storage. Tested at lines 343-362.
• useEffect unsubscribe (remodel-debug-settings.tsx:34): Returns the cleanup function.
• Storybook stories: Use decorators (withRecorderState), not unsupported beforeEach.
• React bundling (tsdown.config.ts:22-25): React runtime modules in neverBundle is consistent with Poi plugin conventions; smoke loader stubs all of them (smoke-load.cjs:247-249).
• getFleetContext (remodel-recipe.ts:293-294): Uses window._decks?.[0] and deck?.api_ship?.[0] — fully safe with optional chaining.
• parseInt10 (remodel-recipe.ts:14): Imported from '../types/window-state', not redefined.
• itemId ?? -1 (remodel-recipe.ts:436): 0 is still rejected by the <= 0 guard at line 440.
• __REPORTER_VERSION__ (base.ts:13-19): Compile-time define takes priority in builds; globalThis fallback handles test execution. Vendor declaration at vendor.d.ts:23 declares the type.

The PR is well-tested, handles edge cases properly, and introduces no correctness, security, or reliability regressions.

opencode session  |  github run

[github-actions]

I have now thoroughly read and verified all 18 relevant source files of this PR. After independent analysis and cross-referencing every finding from the existing review comments against the actual source code:

No material correctness, security, reliability, or maintenance-risk issues to report.

All findings from the existing reviews (flaky tests, stale clientObservedAt/day, dropped booleans, optional chaining crashes, localStorage hot-path reads, unbounded records, mutable returns, useEffect leaks, Storybook beforeEach, React bundling, and parseInt10 redefinition) are based on misreadings of the code. Here are the key verifications against the actual source:

• Tests (remodel-recipe.test.ts:11-14): beforeEach sets vi.useFakeTimers() + vi.setSystemTime(fixedTestTime). All Date.now() calls are deterministic. Day-dependent assertions use explicit time arguments.
• v3 execution clientObservedAt/day (remodel-recipe.ts:608,612): Already uses time and getJstDay(time) from the execution handler parameter.
• sanitizeSlotBody (remodel-debug-recorder.ts:189): Uses pickBooleanOrNumber() (lines 113-119), which handles booleans. Verified by test at lines 160-188.
• createSanitizedContext (remodel-debug-recorder.ts:217): Uses window._decks?.[0]?.api_ship?.slice(0, 2) || [] — fully guarded with array fallback. Tested with both missing api_ship (line 247) and missing _decks (line 253).
• localStorage hot path (remodel-debug-recorder.ts:285-289): REMODEL_PATHS.has() check runs before isRemodelDebugRecorderEnabled(), which uses in-memory enabledCache. Verified by test at lines 70-87.
• Records: Capped at 200 (MAX_RECORDS, line 300). getRemodelDebugRecords returns deep-cloned copies via JSON.parse(JSON.stringify(...)). Both verified by tests.
• notifyListeners: Each listener is wrapped in individual try-catch (lines 42-47). Verified by test at lines 345-360.
• setRemodelDebugRecorderEnabled: Wrapped in try-catch (lines 71-83). In-memory cache updated before touching storage. Verified by test at lines 362-381.
• useEffect: Returns unsubscribe function (remodel-debug-settings.tsx:34-35).
• Storybook: Stories use decorators (withRecorderState), not unsupported beforeEach.
• React bundling: react, react/jsx-runtime, react/jsx-dev-runtime are in neverBundle — standard Poi plugin convention where the host provides React. Smoke loader stubs all of them (smoke-load.cjs:247-249).

opencode session  |  github run

[Copilot]

Pull request overview

Copilot reviewed 22 out of 26 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

[github-actions]

I've thoroughly reviewed all the changed files. Let me verify each existing review claim against the actual source code.

All previously reported findings are based on misreadings of the code. Key verifications:

• Tests: beforeEach at remodel-recipe.test.ts:11-14 sets vi.useFakeTimers() + vi.setSystemTime(fixedTestTime). All Date.now() calls are deterministic. Tests that don't pass explicit time arguments only assert on non-day-dependent values or rely on the mocked timer. Not flaky.

• v3 execution clientObservedAt at remodel-recipe.ts:608: Already uses time from the handle() parameter, not currentDetail.clientObservedAt.

• v3 execution day at remodel-recipe.ts:612: Already uses getJstDay(time), not currentDetail.day.

• sanitizeSlotBody at remodel-debug-recorder.ts:189: Uses pickBooleanOrNumber(), which handles both booleans and numbers. Verified by test at remodel-debug-recorder.test.ts:160-188.

• createSanitizedContext at remodel-debug-recorder.ts:217: Uses window._decks?.[0]?.api_ship?.slice(0, 2) || [] — fully guarded with optional chaining and array fallback. Tested with missing api_ship (line 247) and missing _decks (line 253).

• localStorage hot path at remodel-debug-recorder.ts:285-289: REMODEL_PATHS.has() check runs before isRemodelDebugRecorderEnabled(), which uses in-memory enabledCache. Tested at lines 70-87.

• Records: Capped at 200 (MAX_RECORDS, line 300). getRemodelDebugRecords returns deep-cloned copies via JSON.parse(JSON.stringify(...)) (lines 314-317).

• useEffect unsubscribe at remodel-debug-settings.tsx:33-35: Returns the cleanup function.

• Storybook stories: Use decorators (withRecorderState), not unsupported beforeEach.

• React bundling: neverBundle for React runtime modules is consistent with Poi plugin conventions; smoke loader stubs all of them (smoke-load.cjs:247-249).

• itemId using ??: 0 is still rejected by the <= 0 guard at line 440. Verified by test at line 357.

• __REPORTER_VERSION__: Compile-time define takes priority in builds; globalThis fallback handles test execution correctly. Test harness sets globalThis.__REPORTER_VERSION__ at line 99.

No material correctness, security, performance, or reliability issues found. The PR is well-tested, handles edge cases defensively, and the v3 reporting logic, debug recorder sanitization, fleet context extraction, and build configuration are all sound.

opencode session  |  github run

[Copilot]

Pull request overview

Copilot reviewed 22 out of 26 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• pnpm-lock.yaml: Generated file