Add a useDebouncedSave hook and delete Settings.stream
Two items from Stage 2 of `plan/17-refactor.md`. `frontend/src/hooks/useDebouncedSave.js` replaces three copies of the same debounce. `PlotPanel` and `ScenarioEditor` held identical per-key timer maps. `ScriptEditor` held a single shared timer, so editing two fields inside the same 600 ms window canceled the first save. The hook gives every key its own timer, which fixes that. `Settings.stream` was dead state. Nothing read it and every turn streams. This removes the column, both schema fields, and adds migration 65 to drop it. It is item S1 in `docs/self-review.md`. Migration 65 needs a new guard. `_column_already_gone` is the counterpart to `_column_already_there`: `create_all` builds the current schema, which is already missing every dropped column, so a fixture that stamps an old version and replays would fail on a column that is not there. Verified: 549 backend tests pass, lint and build are clean, and migration 65 runs both ways, once against a database that still has the column and once against one that does not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Dix4oGV3njgWRdu7P9t6r
This commit is contained in:
co-authored by
Claude Opus 5
parent
2c57b1ceab
commit
e0bf2b61d9
@@ -297,6 +297,10 @@ MIGRATIONS: list[tuple[int, str | dict[str, str]]] = [
|
|||||||
# under.
|
# under.
|
||||||
(63, "ALTER TABLE actions ADD COLUMN parent_id INTEGER REFERENCES actions(id) ON DELETE SET NULL"),
|
(63, "ALTER TABLE actions ADD COLUMN parent_id INTEGER REFERENCES actions(id) ON DELETE SET NULL"),
|
||||||
(64, "CREATE INDEX IF NOT EXISTS ix_actions_parent ON actions (parent_id)"),
|
(64, "CREATE INDEX IF NOT EXISTS ix_actions_parent ON actions (parent_id)"),
|
||||||
|
# Phase 17: `Settings.stream` was dead state. Nothing ever read it, and every
|
||||||
|
# turn streams. This is item S1 in `docs/self-review.md`. The table holds one
|
||||||
|
# row per user, so the rewrite is small and needs no VACUUM FULL.
|
||||||
|
(65, "ALTER TABLE settings DROP COLUMN stream"),
|
||||||
]
|
]
|
||||||
|
|
||||||
LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1)
|
LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1)
|
||||||
@@ -427,6 +431,25 @@ def _for_dialect(sql: str | dict[str, str], dialect: str) -> str:
|
|||||||
# Matches the ADD COLUMN migrations in this file. Every one is written above,
|
# Matches the ADD COLUMN migrations in this file. Every one is written above,
|
||||||
# so this pattern parses only SQL this file controls.
|
# so this pattern parses only SQL this file controls.
|
||||||
_ADD_COLUMN = re.compile(r"^\s*ALTER\s+TABLE\s+(\w+)\s+ADD\s+COLUMN\s+\"?(\w+)\"?", re.I)
|
_ADD_COLUMN = re.compile(r"^\s*ALTER\s+TABLE\s+(\w+)\s+ADD\s+COLUMN\s+\"?(\w+)\"?", re.I)
|
||||||
|
_DROP_COLUMN = re.compile(r"^\s*ALTER\s+TABLE\s+(\w+)\s+DROP\s+COLUMN\s+\"?(\w+)\"?", re.I)
|
||||||
|
|
||||||
|
|
||||||
|
def _column_already_gone(conn, sql: str) -> bool:
|
||||||
|
"""Returns `True` when `sql` drops a column the table no longer has.
|
||||||
|
|
||||||
|
This is the counterpart to `_column_already_there`, for the same reason.
|
||||||
|
`create_all` builds the current schema, which is already missing every
|
||||||
|
column a migration drops. Replaying from an older stamp against a database
|
||||||
|
built that way would fail on a column that is not there.
|
||||||
|
"""
|
||||||
|
match = _DROP_COLUMN.match(sql)
|
||||||
|
if match is None:
|
||||||
|
return False
|
||||||
|
table, column = match.group(1), match.group(2)
|
||||||
|
inspector = inspect(conn)
|
||||||
|
if table not in inspector.get_table_names():
|
||||||
|
return False
|
||||||
|
return column not in {col["name"] for col in inspector.get_columns(table)}
|
||||||
|
|
||||||
|
|
||||||
def _column_already_there(conn, sql: str) -> bool:
|
def _column_already_there(conn, sql: str) -> bool:
|
||||||
@@ -946,7 +969,8 @@ def bootstrap(engine: Engine) -> None:
|
|||||||
statement = _for_dialect(sql, conn.dialect.name)
|
statement = _for_dialect(sql, conn.dialect.name)
|
||||||
# Skip the DDL when it has already run. The data pass below it
|
# Skip the DDL when it has already run. The data pass below it
|
||||||
# still runs.
|
# still runs.
|
||||||
if not _column_already_there(conn, statement):
|
if not (_column_already_there(conn, statement)
|
||||||
|
or _column_already_gone(conn, statement)):
|
||||||
conn.execute(text(statement))
|
conn.execute(text(statement))
|
||||||
if version == WORLD_DELTA_VERSION:
|
if version == WORLD_DELTA_VERSION:
|
||||||
_backfill_world_delta(conn)
|
_backfill_world_delta(conn)
|
||||||
|
|||||||
@@ -619,7 +619,6 @@ class Settings(Base):
|
|||||||
"for the player's next action."
|
"for the player's next action."
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
stream: Mapped[bool] = mapped_column(Boolean, default=True)
|
|
||||||
# Phase 6: auto-summarization + memory bank
|
# Phase 6: auto-summarization + memory bank
|
||||||
summary_model: Mapped[str] = mapped_column(String(200), default="") # "" = main model
|
summary_model: Mapped[str] = mapped_column(String(200), default="") # "" = main model
|
||||||
embedding_model: Mapped[str] = mapped_column(String(200), default="") # "" = bank disabled
|
embedding_model: Mapped[str] = mapped_column(String(200), default="") # "" = bank disabled
|
||||||
|
|||||||
@@ -449,7 +449,6 @@ class SettingsOut(ORMModel):
|
|||||||
reasoning_max_tokens: int
|
reasoning_max_tokens: int
|
||||||
context_token_budget: int
|
context_token_budget: int
|
||||||
narrator_prompt: str
|
narrator_prompt: str
|
||||||
stream: bool
|
|
||||||
summary_model: str
|
summary_model: str
|
||||||
embedding_model: str
|
embedding_model: str
|
||||||
memory_bank_capacity: int
|
memory_bank_capacity: int
|
||||||
@@ -496,7 +495,6 @@ class SettingsUpdate(BaseModel):
|
|||||||
reasoning_max_tokens: Annotated[int, Field(ge=-1, le=100_000)] | None = None
|
reasoning_max_tokens: Annotated[int, Field(ge=-1, le=100_000)] | None = None
|
||||||
context_token_budget: Annotated[int, Field(ge=256, le=200_000)] | None = None
|
context_token_budget: Annotated[int, Field(ge=256, le=200_000)] | None = None
|
||||||
narrator_prompt: Prose | None = None
|
narrator_prompt: Prose | None = None
|
||||||
stream: bool | None = None
|
|
||||||
summary_model: Name | None = None
|
summary_model: Name | None = None
|
||||||
embedding_model: Name | None = None
|
embedding_model: Name | None = None
|
||||||
memory_bank_capacity: Annotated[int, Field(ge=1, le=1000)] | None = None
|
memory_bank_capacity: Annotated[int, Field(ge=1, le=1000)] | None = None
|
||||||
|
|||||||
+10
-3
@@ -125,12 +125,19 @@ break compatibility. This is documented in `engine.py`'s prelude instead. The cl
|
|||||||
|
|
||||||
## Cleanup backlog (reuse / simplification / efficiency / altitude — not bugs, apply later)
|
## Cleanup backlog (reuse / simplification / efficiency / altitude — not bugs, apply later)
|
||||||
|
|
||||||
- **R1** `frontend/src/pages/Play.jsx:26` + `ScenarioEditor.jsx:19` + `ScriptEditor.jsx:34`: three copies of the debounced-autosave and story-card handlers. Extract a `useDebouncedSave` hook and a shared StoryCardList component. Fixing bugs #15/#16 properly may accomplish this.
|
- **R1** ~~three copies of the debounced-autosave handler.~~
|
||||||
|
**Half applied in phase 17 (2026-08).** All three use
|
||||||
|
`frontend/src/hooks/useDebouncedSave.js`. A shared StoryCardList component is
|
||||||
|
still open.
|
||||||
- **R2** `backend/seed_demo.py:228`: re-implements create_adventure. Call the router logic instead.
|
- **R2** `backend/seed_demo.py:228`: re-implements create_adventure. Call the router logic instead.
|
||||||
- **R3** `backend/app/providers/openai_compatible.py:122`: complete() duplicates _request()'s body building. Add a `stream` param to _request().
|
- **R3** `backend/app/providers/openai_compatible.py:122`: complete() duplicates _request()'s body building. Add a `stream` param to _request().
|
||||||
- **R4** `backend/app/routers/adventures.py:476`: six copies of child-resource get+owner-check+404. Extract `get_owned_or_404`.
|
- **R4** ~~six copies of child-resource get, owner-check, and 404.~~
|
||||||
|
**Applied in phase 17 (2026-08).** All 32 handlers take the `current_adventure`
|
||||||
|
dependency from `routers/adventures/deps.py`.
|
||||||
- **R5** `frontend/src/api.js:26`: streamSSE duplicates request()'s error extraction. Extract `throwIfNotOk(resp)`.
|
- **R5** `frontend/src/api.js:26`: streamSSE duplicates request()'s error extraction. Extract `throwIfNotOk(resp)`.
|
||||||
- **S1** `backend/app/models.py:210`: `Settings.stream` is dead state (never read). Delete the column and its schema fields.
|
- **S1** ~~`Settings.stream` is dead state (never read).~~
|
||||||
|
**Applied in phase 17 (2026-08).** The column, both schema fields, and migration 65
|
||||||
|
drop it. `R4` went with it: the ownership check is the `current_adventure` dependency.
|
||||||
- **S2** `frontend/src/pages/Play.jsx:6`: MODES and PLAYER_TYPES are identical constants; lastIsAi/canUndo are computed twice.
|
- **S2** `frontend/src/pages/Play.jsx:6`: MODES and PLAYER_TYPES are identical constants; lastIsAi/canUndo are computed twice.
|
||||||
- **E1** `backend/app/context/builder.py:119`: joins and tokenizes the entire adventure history every turn for the trigger window. Walk reversed(actions) until budget instead.
|
- **E1** `backend/app/context/builder.py:119`: joins and tokenizes the entire adventure history every turn for the trigger window. Walk reversed(actions) until budget instead.
|
||||||
- **E2** `backend/app/scripting/pipeline.py:77`: rebuilds full history dicts, JSON, and a blocking commit per script per hook. Build once per hook, slice to HISTORY_WINDOW first, and commit once.
|
- **E2** `backend/app/scripting/pipeline.py:77`: rebuilds full history dicts, JSON, and a blocking commit per script per hook. Build once per hook, slice to HISTORY_WINDOW first, and commit once.
|
||||||
|
|||||||
@@ -0,0 +1,17 @@
|
|||||||
|
// Delays a save until its field has been quiet for a moment.
|
||||||
|
//
|
||||||
|
// Every `key` gets its own timer. One shared timer cancels the pending save of
|
||||||
|
// whatever was edited before it, so editing two fields inside the same window
|
||||||
|
// saves only the second one.
|
||||||
|
|
||||||
|
import { useRef } from 'react'
|
||||||
|
|
||||||
|
function useDebouncedSave(delay = 600) {
|
||||||
|
const timers = useRef(new Map())
|
||||||
|
return (key, fn) => {
|
||||||
|
clearTimeout(timers.current.get(key))
|
||||||
|
timers.current.set(key, setTimeout(fn, delay))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
export { useDebouncedSave }
|
||||||
@@ -1,21 +1,16 @@
|
|||||||
// The plot panel: the adventure's own copy of the scenario text and cards.
|
// The plot panel: the adventure's own copy of the scenario text and cards.
|
||||||
|
|
||||||
import { useRef, useState } from 'react'
|
import { useState } from 'react'
|
||||||
import { api } from '../../../api'
|
import { api } from '../../../api'
|
||||||
import { Field, StoryCardRow, downloadJSON, pickJSONFile, useToast } from '../../../components'
|
import { Field, StoryCardRow, downloadJSON, pickJSONFile, useToast } from '../../../components'
|
||||||
|
import { useDebouncedSave } from '../../../hooks/useDebouncedSave'
|
||||||
import { RefreshModal } from '../RefreshModal'
|
import { RefreshModal } from '../RefreshModal'
|
||||||
|
|
||||||
function PlotPanel({ adventure, setAdventure, onWorldStateChanged }) {
|
function PlotPanel({ adventure, setAdventure, onWorldStateChanged }) {
|
||||||
const toast = useToast()
|
const toast = useToast()
|
||||||
const [plan, setPlan] = useState(null) // non-null while the modal is open
|
const [plan, setPlan] = useState(null) // non-null while the modal is open
|
||||||
const [planning, setPlanning] = useState(false)
|
const [planning, setPlanning] = useState(false)
|
||||||
// One timer per field/card: a single shared timer would cancel the pending
|
const debounceSave = useDebouncedSave()
|
||||||
// save of whatever was edited previously within the debounce window.
|
|
||||||
const saveTimers = useRef(new Map())
|
|
||||||
const debounceSave = (key, fn) => {
|
|
||||||
clearTimeout(saveTimers.current.get(key))
|
|
||||||
saveTimers.current.set(key, setTimeout(fn, 600))
|
|
||||||
}
|
|
||||||
|
|
||||||
const setField = (field, value) => {
|
const setField = (field, value) => {
|
||||||
setAdventure({ ...adventure, [field]: value })
|
setAdventure({ ...adventure, [field]: value })
|
||||||
|
|||||||
@@ -1,7 +1,8 @@
|
|||||||
import { useEffect, useRef, useState } from 'react'
|
import { useEffect, useState } from 'react'
|
||||||
import { useNavigate, useParams } from 'react-router-dom'
|
import { useNavigate, useParams } from 'react-router-dom'
|
||||||
import { api } from '../api'
|
import { api } from '../api'
|
||||||
import { Field, StoryCardRow, downloadJSON, pickJSONFile, useToast } from '../components'
|
import { Field, StoryCardRow, downloadJSON, pickJSONFile, useToast } from '../components'
|
||||||
|
import { useDebouncedSave } from '../hooks/useDebouncedSave'
|
||||||
import ArtPicker from '../ArtPicker'
|
import ArtPicker from '../ArtPicker'
|
||||||
import SchemaEditor, { NpcEditor, addNpc } from '../SchemaEditor'
|
import SchemaEditor, { NpcEditor, addNpc } from '../SchemaEditor'
|
||||||
|
|
||||||
@@ -18,13 +19,7 @@ export default function ScenarioEditor() {
|
|||||||
const [parsedSchema, setParsedSchema] = useState(null)
|
const [parsedSchema, setParsedSchema] = useState(null)
|
||||||
const [schemaView, setSchemaView] = useState('form') // 'form' | 'json'
|
const [schemaView, setSchemaView] = useState('form') // 'form' | 'json'
|
||||||
const toast = useToast()
|
const toast = useToast()
|
||||||
// One timer per field/card: a single shared timer would cancel the pending
|
const debounceSave = useDebouncedSave()
|
||||||
// save of whatever was edited previously within the debounce window.
|
|
||||||
const saveTimers = useRef(new Map())
|
|
||||||
const debounceSave = (key, fn) => {
|
|
||||||
clearTimeout(saveTimers.current.get(key))
|
|
||||||
saveTimers.current.set(key, setTimeout(fn, 600))
|
|
||||||
}
|
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
api.getScenario(id).then((s) => {
|
api.getScenario(id).then((s) => {
|
||||||
|
|||||||
@@ -1,10 +1,11 @@
|
|||||||
import { useEffect, useRef, useState } from 'react'
|
import { useEffect, useState } from 'react'
|
||||||
import { useNavigate, useParams } from 'react-router-dom'
|
import { useNavigate, useParams } from 'react-router-dom'
|
||||||
import CodeMirror from '@uiw/react-codemirror'
|
import CodeMirror from '@uiw/react-codemirror'
|
||||||
import { javascript } from '@codemirror/lang-javascript'
|
import { javascript } from '@codemirror/lang-javascript'
|
||||||
import { oneDark } from '@codemirror/theme-one-dark'
|
import { oneDark } from '@codemirror/theme-one-dark'
|
||||||
import { api } from '../api'
|
import { api } from '../api'
|
||||||
import { Field, downloadJSON } from '../components'
|
import { Field, downloadJSON } from '../components'
|
||||||
|
import { useDebouncedSave } from '../hooks/useDebouncedSave'
|
||||||
|
|
||||||
const SLOTS = [
|
const SLOTS = [
|
||||||
{ key: 'library_js', label: 'Library', hint: 'Shared code prepended to all three hooks.' },
|
{ key: 'library_js', label: 'Library', hint: 'Shared code prepended to all three hooks.' },
|
||||||
@@ -19,7 +20,7 @@ export default function ScriptEditor() {
|
|||||||
const [script, setScript] = useState(null)
|
const [script, setScript] = useState(null)
|
||||||
const [slot, setSlot] = useState('input_js')
|
const [slot, setSlot] = useState('input_js')
|
||||||
const [status, setStatus] = useState('')
|
const [status, setStatus] = useState('')
|
||||||
const saveTimer = useRef(null)
|
const debounceSave = useDebouncedSave()
|
||||||
|
|
||||||
// Test-run state
|
// Test-run state
|
||||||
const [testHook, setTestHook] = useState('input')
|
const [testHook, setTestHook] = useState('input')
|
||||||
@@ -33,12 +34,11 @@ export default function ScriptEditor() {
|
|||||||
|
|
||||||
const setField = (field, value) => {
|
const setField = (field, value) => {
|
||||||
setScript((prev) => ({ ...prev, [field]: value }))
|
setScript((prev) => ({ ...prev, [field]: value }))
|
||||||
clearTimeout(saveTimer.current)
|
debounceSave(field, async () => {
|
||||||
saveTimer.current = setTimeout(async () => {
|
|
||||||
await api.updateScript(id, { [field]: value })
|
await api.updateScript(id, { [field]: value })
|
||||||
setStatus('Saved')
|
setStatus('Saved')
|
||||||
setTimeout(() => setStatus(''), 1500)
|
setTimeout(() => setStatus(''), 1500)
|
||||||
}, 600)
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
const runTest = async () => {
|
const runTest = async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user