Let any turn be played again, not just the newest
`retry` only ever saw the last action: mid-story there was no way to ask for another take at all. Now the take endpoint takes either kind of node, and the kind decides what happens -- an AI turn regenerates, a player turn takes the text supplied. The client asks the same way for both. The tip is the only case that needs no branch, and only for an AI turn, where the takes are still leaves nobody has built on. That is the existing retry, reached by another road. Everywhere else `branch_at` leaves the path just before the turn so the story after it keeps the take it was written for. Both paths roll the shared state back to before the turn ran, which the fork endpoint was already doing and the player-take path was not -- a script would otherwise have stacked this take's output mutations on the one being replaced. 426 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Dvvqn9ZDR4ixeFPHNbww7
This commit is contained in:
committed by
Parth
co-authored by
Claude Opus 5
parent
c3c9b310eb
commit
ea7336e5d3
@@ -1489,15 +1489,18 @@ def add_take(
|
||||
db: Session = Depends(get_db),
|
||||
user: models.User = CurrentUser,
|
||||
):
|
||||
"""Play one of the player's own turns again, differently.
|
||||
"""Play a turn again, differently — whoever wrote it.
|
||||
|
||||
The gap SP7 left: a retry gives an AI turn another take, and nothing gave
|
||||
one to the player's own message. So the only way to change something you
|
||||
had written was to overwrite it and lose the story it led to.
|
||||
One operation for what used to be two and a half. `retry` gave an AI turn
|
||||
another take, but only the newest one; the player's own message had none at
|
||||
all, so changing something you had typed meant overwriting it and losing the
|
||||
story it led to. Here an AI turn regenerates and a player turn takes the
|
||||
text supplied, and neither cares where in the story it sits.
|
||||
|
||||
This is the same move a retry makes, on the other kind of node. The turn is
|
||||
played again with new text; whatever the story did with the old one stays
|
||||
exactly where it is, on the line it was written on.
|
||||
The tip is the only case needing no branch, and only for an AI turn: nothing
|
||||
was played after it, so its takes are still leaves. A player turn is never
|
||||
at the tip — the reply to it is — so it takes a branch every time it has
|
||||
been answered.
|
||||
|
||||
A branch is needed here for the same reason `fork` needs one — the turn
|
||||
being retaken already has a story after it, and that story was written as a
|
||||
@@ -1512,38 +1515,49 @@ def add_take(
|
||||
action = db.get(models.Action, action_id)
|
||||
if action is None or action.adventure_id != adventure_id:
|
||||
raise HTTPException(404, "Action not found")
|
||||
if action.type == "ai":
|
||||
raise HTTPException(
|
||||
400, "Retry gives an AI turn another take; this is for your own."
|
||||
)
|
||||
if action.type not in ("do", "say", "story", "continue"):
|
||||
if action.type not in ("do", "say", "story", "continue", "ai"):
|
||||
# The opening is not a turn anybody played, so there is no second way
|
||||
# to have played it. Editing the scenario is what changes it.
|
||||
raise HTTPException(400, "The opening of a story has no other take.")
|
||||
if action.depth is None or not lineage.path_of(db, adventure).contains(action):
|
||||
raise HTTPException(400, "That turn is not on the story you are reading.")
|
||||
acquire_turn_lock(adventure_id)
|
||||
retry_of = None
|
||||
try:
|
||||
# Only when something was played after it. At the tip there is no story
|
||||
# to protect and no branch is owed — the same rule `stand_on` follows.
|
||||
if adventure.head_depth > action.depth:
|
||||
newest = last_action(adventure, db)
|
||||
at_the_tip = newest is not None and newest.id == action.id
|
||||
if at_the_tip and action.type == "ai":
|
||||
# Nothing was played after it, so its takes are still leaves and a
|
||||
# branch would be for nothing. This is exactly `retry`.
|
||||
retry_of = action
|
||||
attempts.roll_back_before(db, adventure, action)
|
||||
else:
|
||||
# The turn has a story after it, written as a continuation of what
|
||||
# is there now. The new take leaves the path just before the turn,
|
||||
# so that story keeps the take it was written for.
|
||||
tree.branch_at(db, adventure, action.depth - 1)
|
||||
attempts.roll_back_before(db, adventure, action)
|
||||
adventure.updated_at = models.utcnow()
|
||||
db.commit()
|
||||
db.refresh(adventure)
|
||||
except BaseException:
|
||||
_active_turns.discard(adventure_id)
|
||||
raise
|
||||
return StreamingResponse(
|
||||
with_turn_lock(
|
||||
adventure_id,
|
||||
run_player_turn(
|
||||
if action.type == "ai":
|
||||
# No player action to write: the one this turn answers is already on
|
||||
# the path, borrowed from the line being left.
|
||||
stream = generate_turn(
|
||||
adventure, db, ScriptPipeline(adventure, db), user, retry_of=retry_of
|
||||
)
|
||||
else:
|
||||
stream = run_player_turn(
|
||||
adventure,
|
||||
db,
|
||||
schemas.ActionCreate(type=action.type, text=payload.text),
|
||||
user,
|
||||
),
|
||||
),
|
||||
)
|
||||
return StreamingResponse(
|
||||
with_turn_lock(adventure_id, stream),
|
||||
media_type="text/event-stream",
|
||||
headers=SSE_HEADERS,
|
||||
)
|
||||
|
||||
@@ -266,9 +266,15 @@ class ActionCreate(BaseModel):
|
||||
|
||||
|
||||
class TakeCreate(BaseModel):
|
||||
"""Another take of a turn the player wrote themselves (SP9)."""
|
||||
"""Another take of a turn (SP9).
|
||||
|
||||
text: ActionText
|
||||
`text` is what the player is saying instead, and is theirs to write only
|
||||
when the turn was theirs. An AI turn's other take is generated, so the field
|
||||
is ignored there rather than refused — the client asks the same way for both
|
||||
and the node type decides what happens.
|
||||
"""
|
||||
|
||||
text: ActionText = ""
|
||||
|
||||
|
||||
class AdventureOut(ORMModel):
|
||||
|
||||
@@ -358,12 +358,47 @@ def test_both_takes_of_a_players_turn_are_one_group(client):
|
||||
assert _group_size(first.id) == 2, "and reads the same from the other take"
|
||||
|
||||
|
||||
def test_an_ai_turn_is_refused_by_the_take_endpoint(client):
|
||||
def test_an_ai_turn_at_the_tip_takes_no_branch(client):
|
||||
"""Its takes are still leaves. This is `retry`, reached the other way."""
|
||||
_play(client)
|
||||
before = _branch_count(client.adv_id)
|
||||
|
||||
ai = _ai_rows(client.adv_id)[0]
|
||||
r = _take(client, ai.id, "nope")
|
||||
assert r.status_code == 400
|
||||
assert "Retry" in r.json()["detail"]
|
||||
r = _take(client, ai.id, "")
|
||||
assert r.status_code == 200, r.text
|
||||
|
||||
assert _branch_count(client.adv_id) == before
|
||||
assert len(_ai_rows(client.adv_id)) == 2, "a second take, beside the first"
|
||||
|
||||
|
||||
def test_an_ai_turn_the_story_moved_past_takes_a_branch(client):
|
||||
"""Retry could never reach here at all — it only ever saw the newest turn."""
|
||||
_play(client)
|
||||
_play(client, "press on")
|
||||
before = _branch_count(client.adv_id)
|
||||
first_ai = _ai_rows(client.adv_id)[0]
|
||||
|
||||
r = _take(client, first_ai.id, "")
|
||||
assert r.status_code == 200, r.text
|
||||
|
||||
assert _branch_count(client.adv_id) == before + 1
|
||||
# The turn the story moved past now has two takes, and the line it was on
|
||||
# keeps the one it was written for.
|
||||
assert _group_size(first_ai.id) == 2
|
||||
assert first_ai.id in {a.id for a in _ai_rows(client.adv_id)}
|
||||
|
||||
|
||||
def test_the_old_line_still_has_its_continuation(client):
|
||||
_play(client, "open the door")
|
||||
_play(client, "press on")
|
||||
first_ai = _ai_rows(client.adv_id)[0]
|
||||
_take(client, first_ai.id, "")
|
||||
|
||||
# The new take is what this branch tells; "press on" belonged to the other.
|
||||
blob = "\n".join(_path_texts(client))
|
||||
assert "press on" not in blob
|
||||
kept = "\n".join(a.text for a in _user_rows(client.adv_id))
|
||||
assert "press on" in kept, "still there, on the line it was played on"
|
||||
|
||||
|
||||
def test_the_opening_has_no_other_take(client):
|
||||
|
||||
Reference in New Issue
Block a user