Skip to content

feat(kbc): ticket kinds, citation attribution, brief proposals, and cite path prefix - #558

Merged
LikiosSedo merged 7 commits into
mainfrom
feat/kb-proposals
Sep 4, 2026
Merged

feat(kbc): ticket kinds, citation attribution, brief proposals, and cite path prefix#558
LikiosSedo merged 7 commits into
mainfrom
feat/kb-proposals

Conversation

@LikiosSedo

@LikiosSedo LikiosSedo commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Consolidates the Siclaw side of the KB feedback loop onto main (previously stacked as #555 + #558 + #560).

  • Compile-box file_ticket writes contradiction tickets with an evidence-gated ticket_kind (source_conflict vs model_gap), a code-computed claim id, and supersede-on-refile. Ledger backstop stamps handwritten rows unclassified.
  • Gateway persists metadata.knowledge_citations (repo_ids + pages) on the assistant row that rendered those citations, so field feedback can be attributed to the right repo.
  • Box accepts compile.apply_proposal briefs (server-derived shape only) so an approved platform proposal can be executed by the compile agent.
  • knowledge_cite accepts page paths that still carry the knowledge mount prefix (copied from the model's own read call) by stripping leading segments until the path lands on a page read this turn. An unread page still fails as unread.

Not in this PR

Console, proposal queue, Dev MCP, and the effect-loop UI land in the companion platform MR.

Test plan

  • test_selfcheck.py, test_compile_box.py cover the evidence gate, claim-id stability, supersede, residual stamping, and brief-command render
  • knowledge-citations / sse-consumer tests cover attribution passthrough and per-row metadata
  • knowledge-citation-tool covers mount-prefixed relative paths and unread-page rejection
  • After merge: compile of a small repo files typed tickets; a production-style answer persists citation metadata; an approved brief proposal renders in the box

@LikiosSedo LikiosSedo changed the title feat(gateway): persist knowledge-citation attribution on assistant rows feat(kbc): ticket kinds, citation attribution, and brief proposal execution Sep 4, 2026
@LikiosSedo
LikiosSedo changed the base branch from feat/kb-ticket-kind to main September 4, 2026 04:57
LikiosSedo and others added 4 commits September 4, 2026 16:12
…idence-gated kind

Tickets in authoring/CONTRADICTIONS.json were opened by the model writing the
file by hand, with a self-chosen id and no notion of what kind of question a
ticket is. Every consumer downstream (publish gate, UI, the coming proposal
queue) therefore saw one undifferentiated pile of "contradictions".

- New engine tool `file_ticket(ticket_kind, title, question, sources, options,
  current_value, affected_pages)`. The kind is decided by the ticket's own
  evidence, never by the model's say-so: two or more distinct raw documents
  with quotes must be `source_conflict` (the source owner settles it); anything
  less — one source, a BRIEF/tone call, an illegible figure — must be
  `model_gap` (only the owner can answer). A mismatch is refused with the kind
  the evidence supports.
- The id is a code-computed claim fingerprint (normalized question + document
  set). Filing the same claim again supersedes the row — evidence-owned fields
  are replaced, owner/runtime-owned fields (status, answer, agent_report) are
  kept — so a kind can flip as evidence grows without duplicating tickets.
- Post-turn ledger backstop next to the exclusion normalizer: a row written by
  hand gets `ticket_kind: unclassified` (never inferred from sources.length)
  and the owner is told; `selfcheck-residual-*` rows are stamped `model_gap` /
  `origin: selfcheck` by construction.
- Prompts (en/zh box_role, playbook, tools) steer opening through the tool and
  describe the two kinds; the absolute "never edit" wording the R3-2b test
  forbids is avoided — the backstop, not a prohibition, is the guarantee.
- Tests: evidence gate matrix, claim-id stability, supersede semantics,
  normalizer never-guess, residual ticket stamping, allowlist coverage.
…t row

The citation tool already knows which knowledge repo UUID a cited page
belongs to and which answer statement the model bound to it, but only
title/url/page crossed the knowledge_sources event, so a reader's feedback
on an answer could not be traced back to a repo.

- KnowledgeSourceCitation carries `repoId` (from the manifest match) and
  `claim` (the validated pages[].claim); the normalizer passes both through
  with bounds. Rendering is unchanged and URL stays the dedup identity.
- Both gateway consumers (SSE and Lark) write the citations they appended to
  an assistant row onto that row's metadata as `knowledge_citations`
  {repo_ids, pages[{repo_id, page, url, source_id?, evidence?, claim?}]} —
  exactly the delta rendered into that row, so each source lands on one row.
  The platform stores message metadata verbatim and snapshots it onto
  feedback rows, so no wire or schema change is needed for attribution.

Co-authored-by: Cursor <cursoragent@cursor.com>
The platform's proposal queue approves a natural-language repair brief
and dispatches it as a typed authoring command. The box accepts only the
server-derived shape (proposal_id, title, instruction, rationale,
affected_pages — "brief" stays reserved for the compile brief object) and
renders the reviewed text in the run locale, scoped to the named pages, with
the usual source-evidence and file_ticket discipline.

Co-authored-by: Cursor <cursoragent@cursor.com>
…prefix

Models copy paths from their own read calls, so knowledge_cite receives
".siclaw/knowledge/repos/.../page.md" (or "./repos/...") for a page it
demonstrably read this turn. A literal join against knowledgeDir missed and
the cite failed, costing a retry per answer. Resolve by stripping leading
segments until the path lands on a page read this turn; a path that matches
no read page still fails as unread, so stripping never invents a read.

Live traces showed two knowledge_cite calls per answer, the first rejected
for exactly this prefix.

Co-authored-by: Cursor <cursoragent@cursor.com>
@LikiosSedo LikiosSedo changed the title feat(kbc): ticket kinds, citation attribution, and brief proposal execution feat(kbc): ticket kinds, citation attribution, brief proposals, and cite path prefix Sep 4, 2026
knowledge_cite resolves a page at two granularities: a section carrying an
okf:evidence marker cites exactly that marker's sources, a page without
markers cites its whole frontmatter sources (13 originals for one bullet in
the live case). Compiled pages only carried markers where the model wrote
them by hand, and adopted pages carry none.

The compile agent now owns provenance at one level: a (source: X) tag per
statement. At the turn seam, next to the body-citation normalizer and under
the same page scope, selfcheck stamps the machine shapes:

- sources[].id from authoring/manifest.yaml (the frozen manifest ids are the
  only ids the publish gate accepts); a missing id is inserted, a stray one
  replaced, unmanaged resources keep the author's id.
- one marker per answerable section derived from the tags inside it
  (single-source pages bind every section; an untagged section on a
  multi-source page falls back to the page's sources and is reported).
  Machine markers carry the kbc- prefix and regenerate idempotently; an
  authored marker owns its section and is left alone, including the
  "marker directly above the heading" convention the agent already uses.

SELFCHECK.json gains an advisory attribution ledger (sections, attributed,
authored, fallback, ratio, findings, oversized_pages>30 sources). The repair
prompt relays findings while a repair turn is running anyway; nothing gates
on it. Prompts explain that statement tags are what make citations precise
and that ids/markers need not be hand-written.
jacoblee-io
jacoblee-io previously approved these changes Sep 4, 2026

@jacoblee-io jacoblee-io left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all four parts (ticket kinds, citation attribution, brief proposals, cite path prefix). Approving to unblock; 10 findings posted inline — the first three were reproduced concretely against the PR head (two Python repros, one traced resolution path). Most severe: the attribution zip losing page content on masked line-boundary characters, the empty-id: frontmatter corruption the machine then asks the model to repair, and resolveCitedPagePath silently rebinding a never-read page onto an unrelated read page.

Verified but cut by the 10-finding cap: several cleanup/efficiency items, and four candidates were refuted during verification (lark pendingRowCitations reset gap, stale _command_context stamping, stale-CHANGESET harm, pinned-allowlist lockout) — listed so nobody re-raises them.

prose_lines = _markdown_prose(text).splitlines(keepends=True)
body: list[str] = []
prose: list[str] = []
for raw, masked in zip(lines[fm_end + 1:], prose_lines[fm_end + 1:]):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced against this head. _attribute_page_text zips raw splitlines() against _markdown_prose() lines, and the two fall out of alignment when masking erases a line-boundary character (lone \r, \f, \v, \u2028) inside a code span. A candidate page whose fenced block contains pasted terminal output with a stray CR (out\rput) plus a trailing last line: text.splitlines() yields one more line than the masked prose, the zip drops the tail, and attribute_evidence_sections (which runs at every turn seam) writes the page back with last line deleted. Every line after the fence also pairs with the wrong masked line, so markers/headings below can be misclassified. Mechanical, per-turn knowledge-page data loss — the masking needs to preserve line boundaries (or the zip needs to be index-mapped rather than positional).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 5a265d5 — raw and masked lines are now paired by an \n-only split (_lf_lines; \n survives masking, the exotic line-boundary chars don't), plus a defensive length-mismatch bail that leaves the page untouched rather than rewriting against wrong lines. CR-in-fence tail-preservation test added.

Comment thread kbc/platform/pod/selfcheck.py Outdated
indent = " " * key_node.start_mark.column
edits.append((insert_at, insert_at, f"{indent}id: {json.dumps(wanted)}{newline}"))
elif existing != wanted and isinstance(id_pair[1], yaml.ScalarNode):
edits.append((id_pair[1].start_mark.index, id_pair[1].end_mark.index, json.dumps(wanted)))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced against this head. _stamp_source_ids_text corrupts frontmatter when a managed source row carries a bare empty id: — the null scalar's zero-width span sits immediately after the colon, so the replacement inserts id:"uuid" with no space, which is invalid YAML. Page with - resource: raw/a.md + bare id: and a.md in the manifest → existing=None != wanted takes the replace branch, json.dumps(wanted) is spliced at the empty scalar's mark producing id:"0612b522-…"; yaml.safe_load then fails ("while scanning a simple key"). The machine writes the corrupted page to disk; on the next turn parse_okf_sources errors and the model receives repair directives for corruption the system itself wrote. Insert a space (or normalize the empty scalar) before splicing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 5a265d5 — a zero-width null-scalar span (or a splice point directly after :) now gets a space prepended, so a bare id: becomes valid id: "uuid".

Comment thread src/core/knowledge-citation-tool.ts Outdated
const literal = path.resolve(path.isAbsolute(trimmed) ? trimmed : path.join(knowledgeDir, trimmed));
if (wasRead(literal) || path.isAbsolute(trimmed)) return literal;
const segments = path.posix.normalize(trimmed).split("/").filter((segment) => segment !== "" && segment !== ".");
for (let i = 1; i < segments.length; i++) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveCitedPagePath strips leading path segments until ANY suffix matches a read page, so a cite naming a genuinely different, never-read page silently rebinds to an unrelated read page instead of failing "Cannot cite unread knowledge page" — corrupting exactly the feedback-attribution metadata this PR introduces. Model read knowledgeDir/guide.md (repo A) this turn, then cites repos/kb-2/guide.md (repo B's page, never read, or a hallucinated path): the literal join misses readPages, the loop strips down to guide.md which WAS read, and the cite succeeds against repo A — the rendered reference, repoId, and claim persisted in knowledge_citations all name the wrong repo/page. The stripping isn't limited to known mount prefixes (.siclaw/knowledge, ./) — any leading segments are dropped. Old code failed closed and let the model correct itself. Root cause worth noting: agent-factory records reads under the model's own spelling rather than a canonical form; limiting the strip to known mount prefixes would keep the fix without the rebinding.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 416d91b — the strip is now limited to a MOUNT prefix (.siclaw/knowledge/ or the trailing segments of knowledgeDir), so repos/other/guide.md / ../guide.md stay unread and fail closed instead of rebinding. Exactly the suggested shape; tests added.

Comment thread kbc/platform/pod/selfcheck.py Outdated
raw documents it is about. Computed by code so two compile rounds asking
the same thing land on the same ticket (supersede, not duplicate), while
the kind is free to change as evidence changes."""
docs = sorted({row["doc"] for row in sources})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ticket_claim_id hashes question + doc SET, so the advertised flagship supersede ("a single-source model_gap flips to source_conflict when a later round finds the second document, without a duplicate row") is impossible — finding the second document changes the id and files a duplicate while the stale model_gap stays open. Round 1: one doc quotes "52 nodes" → model_gap tk-A. Round 2 finds a disagreeing second doc; re-filing the same question with both quotes computes a NEW id → source_conflict tk-B appended (the PR's own test test_compile_box.py:7582 asserts the ledger holds TWO rows for one question). model_gap never auto-closes, and box_role.md only tells the model to re-check tickets whose sources include a CHANGED source (docA didn't change) — so the owner answers stale tk-A with outdated options while the source owner separately resolves tk-B. Sub-bug: compile_box.py computes the tid over the full sources list but persists sources[:16], so a >16-source ticket re-filed from its stored row also changes id and duplicates. Hash the question alone (or implement the supersede as an explicit id lookup).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially fixed in 416d91b: the sub-bug is closed — file_ticket now truncates question/sources BEFORE computing the tid, so a >16-source ticket re-filed from its stored row keeps its id (and the normalizer's fingerprint check re-derives it exactly). The main issue stands: ticket_claim_id still hashes question + doc set, so the advertised model_gap→source_conflict supersede (finding the second document) still changes the id and files a duplicate while the stale model_gap stays open.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Main issue also fixed in 5a265d5ticket_claim_id now hashes the question alone (sources accepted for call-site compatibility, deliberately not hashed), so finding the disagreeing second document supersedes the model_gap in place instead of filing a duplicate. In-place supersede test added; the round-2 fingerprint check still works since sources are ignored.

Comment thread kbc/platform/pod/selfcheck.py Outdated
if not isinstance(t, dict):
continue
tid = str(t.get("id", ""))
if not t.get("ticket_kind"):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ticket-kind evidence gate exists only inside the file_ticket tool; normalize_contradictions_file stamps rows MISSING ticket_kind but never validates a kind that is PRESENT — so a hand-written row with a self-chosen (or invalid) ticket_kind bypasses the gate the PR says the model "cannot talk its way past". A model refused by file_ticket (kind_needs_gap) Writes a row with "ticket_kind": "source_conflict" and one source directly into authoring/CONTRADICTIONS.json; the post-turn backstop branches on if not t.get("ticket_kind") and leaves it untouched, the owner is never told, and the mis-kinded ticket enters the source-owner routing / auto-close flow. The module comment's claim that kind is "re-decided every compile round from the evidence on the ticket itself" is implemented nowhere — no per-round pass re-runs ticket_kind_allowed_by_evidence over existing rows. Run the gate over PRESENT kinds in the backstop too.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 416d91bnormalize_contradictions_file no longer trusts a PRESENT ticket_kind: a row counts as tool-filed only if its id equals the claim fingerprint re-derived from the row's own (bounded) question + normalized sources; anything else is stamped unclassified. selfcheck-residual-* rows are forced to model_gap, edge-origin rows left alone. box_role en/zh updated to match.

if existing and entry:
ids[entry] = existing
continue
ids[entry] = wanted

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced against this head. When a source row's id value is a non-scalar YAML node (e.g. id: [x]), _stamp_source_ids_text records the manifest id in the returned ids map without editing the text — so _attribute_page_text emits machine markers citing a source id the page never declares. Page with - resource: raw/a.md + id: [x] → ids map claims {'a.md': manifest-uuid} but the replace branch requires a ScalarNode so the frontmatter keeps [x]; the emitted marker {"sources":["<uuid>"]} fails the "cites an id not declared in sources" lint. The prompts forbid the model from writing okf:evidence markers or sources[].id itself, and the marker is regenerated every pass, so the repair loop can never converge. Either skip the ids-map entry when the text wasn't edited, or replace the non-scalar node too.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 5a265d5 — a non-scalar id row is now left out of the ids map entirely, so the emitted marker never cites an id the frontmatter doesn't declare; the OKF lint on the row tells the model what to fix, and the loop converges.

return out


def ticket_distinct_raw_docs(sources: list[dict]) -> list[str]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced against this head. ticket_distinct_raw_docs counts two spellings of one document as two distinct docs (only the exact raw//drop/ prefix is unified; no basename/path unification, and only the literal authoring/ prefix marks non-raw evidence), so the §D3 gate can FORCE source_conflict on a ticket with no actual source dispute. [{doc:'manual.md'},{doc:'docs/manual.md'}] (basename spelling the body-tag convention elsewhere accepts via basename fallback) → 2 "distinct raw docs" → file_ticket refuses the correct model_gap and instructs resubmission as source_conflict. Likewise [{doc:'BRIEF.json'},{doc:'a.md'}] — the en tools.json itself says "a tone/scope call based on BRIEF.json" without the authoring/ prefix — forces source_conflict for a tone question, routing an owner-judgment ticket to the source-owner flow where "not re-detected" auto-close can silently close it unanswered. Normalize by basename (matching the body-tag fallback) and recognize BRIEF.json regardless of prefix.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially fixed in 416d91b: normalize_ticket_sources now normpaths and unifies backslashes, so a.md / ./a.md / raw/./a.md are one document. Still open: basename vs pathed spellings (manual.md vs docs/manual.md — the spelling the body-tag convention accepts via basename fallback) still count as two distinct raw docs, and a bare BRIEF.json (the spelling the en tools.json itself uses) still isn't matched by the literal authoring/ prefix — both §D3 forced-conflict repros survive.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Remaining cases fixed in 5a265d5_TICKET_NON_RAW_BASENAMES recognizes authoring framing (BRIEF.json etc.) by basename wherever spelled, and a bare basename unifies with the single pathed row it matches (ambiguous basenames and two pathed rows sharing a basename correctly stay distinct). Both §D3 repros are covered by tests now.

"compile.repair_test",
# Full-library domain write/update only — not a scoped incremental turn.
"compile.refresh_domain",
# An APPROVED brief proposal (KB feedback loop): a scoped repair the owner

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compile.apply_proposal validates affected_pages but uses them ONLY to render prompt text — unlike incremental recompile it never arms the _incr_pending sha256 byte-integrity guard, so "Touch only these candidate pages" is honor-system for an owner-approved, scoped repair. Owner approves a proposal scoped to pages A and B; the model also "improves" unrelated pages during the turn; nothing restores them (the byte-exact restore backstop is armed only by _start_incremental), the turn runs with grandfather_legacy_format=True, and unreviewed edits ship under an approval that named only A and B — the exact drift the incremental path's guard exists to prevent. Arm the same guard from the proposal's affected_pages.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjacent hardening in 416d91b (brief/renew refused + unknown keys dropped — closes a BRIEF.json rewrite vector, good), but the finding itself stands: affected_pages still only renders prompt text and the _incr_pending sha256 byte-integrity guard is still not armed, so out-of-scope page edits during an approved proposal run still ship unreviewed and unrestored.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 5a265d5_arm_proposal_scope_guard snapshots page hashes/bytes and arms _incr_pending (scope "proposal") from affected_pages before the apply_proposal turn, same closing gate as an incremental round; out-of-scope edits are restored byte-exact.

Comment thread src/core/knowledge-citation-tool.ts Outdated
resolveCitedPagePath(value, opts.knowledgeDir, (absolute) => readPages.has(absolute)));
// The validated claim rides on the citation for attribution (feedback →
// which statement, which page); it is not rendered.
const claimByPage = new Map(selected.map((page, i) => [page, pageArgs[i].claim]));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claimByPage is keyed by resolved absolute page, so two pages entries that resolve to the same page — a duplication resolveCitedPagePath now actively legitimizes (guide.md and .siclaw/knowledge/guide.md) — keep only the LAST claim. pages: [{path:'guide.md', claim:'statement A'}, {path:'.siclaw/knowledge/guide.md', claim:'statement B'}]: nothing rejects duplicates (no uniqueItems, no dedup in the validation loop); both resolve to one absolute key and the Map keeps claim B only, so the knowledge_citations metadata attributes the page's sources solely to B and downstream feedback for claim A points at nothing. Rendering is unaffected (URL dedupe) — only the attribution record loses a claim. Either reject resolved-duplicate paths or accumulate claims per page.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 5a265d5 — claims accumulate per resolved page (previous | next) instead of last-writer-wins, so feedback on the first statement no longer points at nothing.

are ATX-heading spans plus the preamble; a marker whose next non-blank line
is a heading belongs to that heading's section (the compile agent's own
convention), so an authored marker keeps guarding the section below it.
Machine markers are emitted on the line directly above the heading (top of

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Machine markers are placed on the line directly ABOVE the heading (next-heading-owns-it convention), but the answering model's system prompt only says to use evidence_refs "for sections that contain an okf:evidence marker" and never states that convention — textually the marker sits inside the PRECEDING section. The runtime resolves refs by id lookup alone (pageProvenanceFromBody), so there's no mechanical mis-binding, but a literal-minded answering model reading a stamped page sees kbc-2's marker as the last line of section 1 and can cite section 1's facts with section 2's sources, or treat the section below as unmarked — silently, since id-only resolution can't detect it. The above-heading rule is documented only compile-side (design doc + selfcheck docstring), nowhere the answering model sees. Also: setext-headed pages collapse to a single page-top marker (the heading regex is ATX-only) while the answering model sees several sections. State the convention in the answering-side prompt.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in 5a265d5 — the answering-side prompt now states the convention ("a marker on the line directly above a heading belongs to the section under that heading; a marker before the first heading covers the page's introduction"), and setext headings are recognized as section boundaries in _attribute_page_text (with the underline excluded from section content). Design doc updated.

…ticket kinds, apply_proposal and Lark rollback

- knowledge_cite strips only a MOUNT prefix (`.siclaw/knowledge/` or the
  trailing segments of knowledgeDir). The segment-by-segment strip could bind
  `repos/other/guide.md` or `../guide.md` to a read `guide.md`, writing
  attribution against the wrong page. Unread spellings now stay unread.
- Hand-written tickets are unclassified whatever ticket_kind they type: a row
  is the tool's only if its id equals the claim fingerprint file_ticket
  derives from the row's own question and document set (computed from the
  stored, bounded values so the normalizer re-derives it exactly). Doc paths
  are normpath'ed so `a.md` / `./a.md` / `raw/./a.md` are one claim.
- compile.apply_proposal refuses brief/renew and keeps only the reviewed
  keys, so a proposal execution cannot rewrite authoring/BRIEF.json.
- Lark: on model_route_rollback the already-persisted primary row is marked
  discarded with its knowledge_citations removed, pendingRowCitations is
  cleared, and the fallback answer gets its own row — matching what the SSE
  consumer achieves by buffering.

@jacoblee-io jacoblee-io left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round-2 verification of 416d91b (details in the per-comment replies):

Fixed — cite resolution now strips only mount prefixes (unread spellings fail closed); hand-written tickets are fingerprint-checked so a typed ticket_kind can't bypass the gate; the sources[:16] id-drift sub-bug is closed (tid computed from stored values).

Partially fixed — doc-spelling unification covers normpath cases but manual.md vs docs/manual.md and bare BRIEF.json still force source_conflict (§D3 repros survive); apply_proposal now refuses brief/renew (good hardening) but still doesn't arm the _incr_pending sha256 guard.

Still open, including the two most severe (both reproduced against the head):

  1. _attribute_page_text zip misalignment — silent per-turn knowledge-page tail loss when masking erases a line-boundary char inside a code span (selfcheck.py:2157).
  2. Empty id: frontmatter corruption — the machine splices id:"uuid" with no space, writes invalid YAML, then directs the model to repair it (selfcheck.py:2098).
  3. ticket_claim_id hashing the doc set — the advertised model_gap→source_conflict supersede still can't happen; finding the second doc files a duplicate.
  4. Non-scalar id → markers citing an undeclared source id, a never-converging lint loop (selfcheck.py:2089).
  5. claimByPage last-claim-wins on resolved-duplicate pages; and the above-heading marker convention still isn't stated anywhere the answering model sees.

Holding approval until at least #1 and #2 (the mechanical data-corruption pair) are addressed.

- Attribution pairs raw and masked lines on "\n" only; str.splitlines also
  split on lone CR/FF/VT/U+2028 that masking turns into spaces, so the zip
  slid and the page tail was dropped at every turn seam (reproduced). A
  mismatch now leaves the page untouched instead of rewriting it.
- Source-id stamping puts a space before the value spliced into a bare
  `id:` (it wrote `id:"…"`, invalid YAML the model was then asked to repair)
  and no longer records an id for a non-scalar `id: [x]` row, so a marker
  never cites an id the frontmatter does not declare.
- ticket_claim_id hashes the question alone: finding the disagreeing second
  document now supersedes the model_gap in place instead of filing a
  duplicate beside it. ticket_distinct_raw_docs treats a bare basename as
  the pathed row it matches and recognizes authoring/ framing (BRIEF.json…)
  by basename, so neither spelling can force source_conflict.
- compile.apply_proposal arms the byte-integrity guard from affected_pages;
  out-of-scope edits during an approved brief run are restored, not shipped.
- knowledge_cite accumulates claims when two entries resolve to one page.
- Setext headings are sections too; the answering prompt states the
  marker-above-heading convention.

Tests: CR-in-fence tail preservation + setext; bare/non-scalar ids; in-place
supersede; basename/BRIEF.json spellings; scope guard armed; claim merge.

@jacoblee-io jacoblee-io left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving after 5a265d5. All 10 round-1 findings are now closed and verified against the diff (per-comment replies have the details): the two reproduced data-corruption bugs (zip misalignment via \n-only pairing + length-mismatch bail; bare-id: splice spacing), the non-scalar-id lint loop, question-only ticket_claim_id enabling in-place supersede, basename/BRIEF.json doc unification, the apply_proposal byte-integrity guard, claim accumulation on duplicate-resolved pages, and the marker convention stated answering-side with setext support. Each fix has a matching test; CI green across all five checks. Nothing outstanding from my side — good to merge.

@LikiosSedo
LikiosSedo merged commit 7b23d66 into main Sep 4, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants