feat: the bible tools can correct what they seeded, not only add to it #168
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/bible-tools-correct-entries"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
An agent's friction report from a real bible-building run: story beats and glossary entries could only be appended. A seeded beat with a detail none of the transcribed regions support stayed wrong in the shared bible. A glossary target mis-entered as the text
nullcould only be "fixed" by adding a second copy, so アクメ ended up with two entries. Characters and lore already had replace-by-key upserts (ADR 0016); the glossary and the beats did not.What's in
Glossary —
add_glossary→upsert_glossary(BibleTools.cs)upsert_characterandupsert_loredo by name and title. It is renamed so the tool name says it replaces, the way its two siblings' names do.target: if omitted, the settled rendering is kept, so a pass that refines the note cannot erase what translation decided;""clears it; any other value sets it.Story beats —
update_story_beat(number, text)anddelete_story_beat(number)list_biblenow printsstory_beatsas{number, text}rather than bare strings, so the agent reads the address instead of counting.set_page_summary), so a miscounted number shows which beat it hit in the transcript. A number out of range reads back the valid range.add_story_beatnow reports its number by count, notOrder + 1. A deletion leaves a gap in the stored order, so the old number would have pointed past the beat it named.Grants and prompts
Tests
1,962 passed, 1 skipped (pre-existing): Domain 132, UseCases 676, Integration 875 (+1 skipped), BlazorAdapter 279.
New tests in
AgentToolTests:Upsert_glossary_replaces_by_term_and_keeps_the_rendering_it_was_not_told: re-upserting a term updates the note and keeps the target when it is omitted; an explicit""clears it.Upsert_glossary_collapses_a_terms_duplicates_onto_one_corrected_entry: seeds the reported state (a"null"copy, then a correct copy, plus an unrelated term). One entry survives, carrying the newest copy's rendering; the unrelated term is untouched.A_story_beat_is_rewritten_in_place_by_number_and_quotes_what_it_replaced: the middle beat is rewritten and its neighbours keep their places; a blank rewrite is refused rather than acting as a delete.Deleting_a_story_beat_renumbers_the_rest_and_the_next_add_counts_from_there: after a delete, the next add reports beat 3, andlist_bible's numbers agree.A_story_beat_number_outside_the_list_reads_back_the_range: covers the empty list, a number past the end, and a missing number, with nothing written in any case.Updated tests:
AgentToolSchemaTestspins the required fields:UpsertGlossaryParamsstayssource, note;UpdateStoryBeatParamsisnumber, text;DeleteStoryBeatParamsisnumber.Notes
🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.7%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.3%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.7%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! An append-only bible learning to correct itself~ A friction report about アクメ living twice in the glossary, and out comes a proper replace-by-key family — the glossary promoted to sit beside its two upsert siblings, and the keyless beats given numbers to be addressed by. This is exactly the kind of small slice I adore: one gap, closed all the way through the tools, the grants, the prompts, and the ADR. ♪
Verdict: ✅ Looks good to me~
I came hunting, blades out, and the new code held. ♡
✅ What I liked~
add_story_beatnow numbers by count, notOrder + 1— this is the deep one. A deletion leaves a gap in the stored order, so the old number would have pointed past the beat it named; the number is the address now, and the address space is the list.NextOrderkeeps the stored orders collision-free while the tools speak in dense numbers — two-tier design, correct, andDeleting_a_story_beat_renumbers_the_rest...pins it.targetsemantics — omitted keeps the settled rendering (a refinement pass can't erase what translation decided),""clears it. Both arms tested.EfBibleStore.ListGlossaryAsyncorders byCreatedAt(UTC ticks) and the fake orders by insertion, somatches[^1]is the newest copy in both worlds. The reported アクメ state is seeded verbatim in the test,"null"typo and all. Fufu~set_page_summary; family behavior.StoryBeatByNumber— one resolver shared by both correcting tools, and the out-of-range reply reads back the valid range (empty-list arm included).AgentToolSchemaTests), both grant lists, the provider wire count 13→15, both prompts, and ADR 0016's catalog line. No danglingadd_glossaryanywhere — I looked..OrderBy(b => b.Order)fromlist_bibleis safe:GetBiblealready sorts beats byOrderbefore the DTO mapping. I checked rather than assumed~💡 Little ideas (non-blocking)~
DeleteGlossaryEntry's result. If a duplicate vanished between the read and the delete, the reply still claims "removed N duplicate entries" — the final state is right either way (one corrected entry, and the next upsert re-collapses), so this is only message attribution in a rare race. The codebase does test vanishing-row seams elsewhere (SimulateVanishingPageSummaries); a false-result count-down would make the claim exact, if that race ever bothers you.🧪 Mutation probes — your tests bit me back~
I mutated the riskiest new branches; all three probes died:
StoryBeatByNumber(n >= 0 && n <= Count + 1) → killed byA_story_beat_number_outside_the_list_reads_back_the_range.matches.Take(0)) → killed byUpsert_glossary_collapses_a_terms_duplicates_onto_one_corrected_entry.?? kept.Target) → killed twice, by both upsert-glossary tests.Nothing survived. The new code is guarded, not just compiled~ ♡
Automated review by Jibril · 2026-10-05
CI: green for head SHA
a96395a· Mutation probes: 3 run, 0 survived / 0 skipped · Local suite: skipped (CI green)Thanks for the green on
a96395a. I took the leftover idea ine51b948. It changes production code, so I expect a new round on that head.BibleTools.cs): the loop now counts only the deletes that returnedOk, so the reply mentions only the copies this call actually removed. If every older copy was already deleted by another write, the reply is plainUpdated glossary entry “…”.The resulting glossary is the same either way; only the reply's wording changes.FakeBibleStore.GlossaryDeletedElsewhereset. For an id in that set, the row is removed but the delete returnsfalse, which simulates another write deleting it first.A_duplicate_another_writer_deleted_first_is_not_claimed_as_removed.Full suite: 1,965 passed, 1 skipped (already skipped before this PR). Domain 132, UseCases 679, Integration 875 (+1 skipped), BlazorAdapter 279.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Back so soon~? I barely had time to sheathe my blades and you bring me a second course ♪ A re-review at
e51b948that takes my one little idea and closes it properly — not by silencing the count, but by teaching it to tell the truth. And a test seam for a race I only described last time! Fufu~ you know how to make a Flugel feel seen.Verdict: ✅ Looks good to me~
✅ What I liked~
mergedonly whendeleteEntryreturnsOk<Unit>, so the reply claims exactly the copies this call removed. The pattern type is exact, not coincidental:DeleteGlossaryEntryreturnsOk(Unit.Value)on success andFail("The glossary entry no longer exists.")when the row already vanished — you match the right arm, and a failed delete leavesmergedalone instead of leaking into the wording.FakeBibleStore.GlossaryDeletedElsewheredoesn't fake the outcome; it fakes the race:RemoveAll(...) > 0 && !Contains(id)still deletes the row (final state identical either way) and only the return flips tofalse. That's precisely the read-vs-delete seam from my 💡1, with the XML doc saying exactly that. TheSimulateVanishingPageSummarieslineage continues~matches[..^1]→ "removed 1 duplicate entry" while the survivor carries the newest rendering; then two copies whose only duplicate is raced → plainUpdated glossary entry “先輩”.with nothing claimed. Both assertions land on the wording, which is the only thing this fix changes.merged == 0— the: $"... and removed {merged} ..."ternary keeps zero-removal silent, so the quiet arm stays quiet. ♡🧪 Mutation probe — it bit back, how lovely~
I reverted the counting to the old claim-everything behavior (
merged = matches.Count - 1, deletes still issued) and ran the new test:A_duplicate_another_writer_deleted_first_is_not_claimed_as_removedfailed immediately —Assert.Contains("removed 1 duplicate entry")not found; the mutant boasted "removed 2". Killed. Restored pristine code, re-ran the new test plus allUpsert_glossarytests: 4/4 passed. The delta isn't just compiled, it's guarded~The three probes from my
a96395areview all targeted code and tests unchanged in this delta, so their kills stand (not re-run, per my notes).✨ The PR as a whole~
This remains the family-shaped slice I praised last round — replace-by-key glossary, numbered beats, grants, prompts, and ADR 0016 all moved together. No new blocking or non-blocking findings on this delta.
Automated review by Jibril · 2026-10-05
CI: green for head SHA
e51b948· Mutation probes: 1 run, 0 survived / 3 skipped (unchanged sincea96395areview) · Local suite: skipped (CI green)