Rifqi Hafizuddin Claude Opus 5 commited on
Commit
5cb2e2d
·
1 Parent(s): e2ae28f

[NOTICKET] feat(knowledge): review diff + the rule/domain boundary (r4, o2)

Browse files

r4 `changed_fields()` and `review_diff()`. The queue can now say WHAT moved
rather than only that something did - which was the ask: show before /
after, do not send the expert back to read the entry from the start.
`changed` is empty when the approval still stands, so the screen can say
"nothing to re-read" instead of rendering two identical panes. `before` is
what the expert actually had in front of them; for an `edited` decision
that is their own corrected text, not the model's claim. A test pins
changed_fields() and has_mutated() to the same answer - two notions of
"has this changed" that can disagree is the same defect as two notions of
readiness.

o2 The rule/domain boundary, written down in the output contract. The two
blurred on screen during the 8 Sep review and a boundary nobody has
recorded is one the prompts cannot hold. The test: if removing it would
change how a NUMBER IS COMPUTED OR SOURCED it is a rule; if it would only
leave an agent less oriented it is domain. Grain settles the hard cases
faster than "high level" does - one document's sentence, or the corpus as
a whole. Where it is genuinely both, prefer rule: it carries a span an
expert can check.

Also marks section 6 as EXECUTED (2026-09-14, after the handoff went to Harry -
run by hand AND handed over, in that order) and adds section 7, NOT RUN: the
three columns r1/r2/r3 need. An entry approved with no human looking at it must
still record who allowed that, which is why it belongs in columns rather than a
free-text note.

Verified: ruff clean on both knowledge modules; 808 passed / 7 skipped (was
801+7, +7 new); import main exits 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs/knowledge/KNOWLEDGE_OUTPUT_CONTRACT.md CHANGED
@@ -85,6 +85,38 @@ registry, which belongs with persistence (DEV_PLAN §0.8 D2).
85
 
86
  Plus three derived/audit payloads: `review_queue.json`, `rejected.json`, `usage.json`.
87
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
88
  ### Why `RuleEntry` carries a `rule_type`
89
 
90
  The Interpretation Pack exists to **improve analytics insight from expert interpretation rules** —
 
85
 
86
  Plus three derived/audit payloads: `review_queue.json`, `rejected.json`, `usage.json`.
87
 
88
+ ### Rule vs domain knowledge — where the line is
89
+
90
+ *Added 2026-09-14 (o2). The two blurred on screen during the 2026-09-08 review,
91
+ and a boundary nobody has written down is one the prompts cannot hold.*
92
+
93
+ | | **`rule`** | **`domain`** |
94
+ |---|---|---|
95
+ | Grain | One statement, from one document | One per company, composed across all of them |
96
+ | Shape | A condition and a consequence | Identity, boundary, conventions, measures |
97
+ | Evidence | Span-checked against its source | Derived from entries, or expert-declared |
98
+ | Answers | *"…and how do I compute this correctly?"* | *"…what does this company measure, and what does it call it?"* |
99
+
100
+ **The test to apply.** If removing it would change **how a number is computed
101
+ or sourced**, it is a rule — *"if joint-survey data is unavailable, use truck
102
+ count"* changes the figure. If removing it would only leave an agent **less
103
+ oriented** — not knowing that this company works in overburden and BCM — it is
104
+ domain.
105
+
106
+ Two consequences worth stating, because they are what the distinction is *for*:
107
+
108
+ - **A rule is actionable alone; domain knowledge is not.** A planner can apply
109
+ a rule to a specific calculation. It cannot apply "this is a mining company"
110
+ to anything — that shapes which questions make sense, not which column to sum.
111
+ - **Domain is high-level and rules are specific, but "high-level" is not the
112
+ test.** A long, general rule is still a rule. The grain question — *does this
113
+ come from one document's sentence, or from the corpus as a whole?* — settles
114
+ it faster and does not drift.
115
+
116
+ Where it is genuinely both, prefer the **rule**: it carries provenance and a
117
+ span, so an expert can check it. Domain fields are the ones the pipeline can
118
+ least easily prove.
119
+
120
  ### Why `RuleEntry` carries a `rule_type`
121
 
122
  The Interpretation Pack exists to **improve analytics insight from expert interpretation rules** —
docs/knowledge/knowledge_tables.sql CHANGED
@@ -22,9 +22,10 @@
22
  -- Executed: 2026-09-07 · Idempotent (IF NOT EXISTS throughout) · Additive only:
23
  -- no DROP, no ALTER, nothing touching an existing table.
24
  --
25
- -- ⚠️ Updated 2026-09-14: §1-§5 are LIVE and unchanged. A new **§6 at the end
26
- -- is NOT run anywhere** — it is the column request from the 2026-09-08
27
- -- discussion (m3, m4, m5, m8). Read the banner there before adopting.
 
28
 
29
  -- 1. Parsed document artifact. Chunks live INSIDE `artifact` (jsonb), deliberately
30
  -- not a child table: documents are small (9 pages = 31 chunks) and the artifact
@@ -175,9 +176,11 @@ CREATE INDEX IF NOT EXISTS idx_knowledge_jobs_user ON knowledge_jobs (user_id, c
175
 
176
 
177
  -- ============================================================================
178
- -- §6. NOT RUN ANYWHERE YET — from the 2026-09-08 discussion (m3, m4, m5, m8).
179
  --
180
- -- Everything above this line is live in dedorch. Everything below is a request.
 
 
181
  --
182
  -- Additive only: three columns and one index on `knowledge_entries`. Nothing
183
  -- is dropped and no column changes type, so a pre-migration reader keeps
@@ -228,3 +231,38 @@ CREATE INDEX IF NOT EXISTS idx_knowledge_entries_key ON knowledge_entries (scope
228
  ALTER TABLE knowledge_documents ADD COLUMN IF NOT EXISTS withdrawn_at timestamptz;
229
  CREATE INDEX IF NOT EXISTS idx_knowledge_documents_live
230
  ON knowledge_documents (scope_id, doc_id) WHERE withdrawn_at IS NULL;
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
22
  -- Executed: 2026-09-07 · Idempotent (IF NOT EXISTS throughout) · Additive only:
23
  -- no DROP, no ALTER, nothing touching an existing table.
24
  --
25
+ -- ⚠️ Updated 2026-09-14. §1-§5 LIVE and unchanged. §6 (m3/m4/m5/m8/i7) was
26
+ -- EXECUTED against dedorch on 2026-09-14 by Rifqi, after the handoff had
27
+ -- already been sent to Harry — run by hand AND handed over, in that order,
28
+ -- which is the thing the banner above asks for. §7 is NOT run anywhere yet.
29
 
30
  -- 1. Parsed document artifact. Chunks live INSIDE `artifact` (jsonb), deliberately
31
  -- not a child table: documents are small (9 pages = 31 chunks) and the artifact
 
176
 
177
 
178
  -- ============================================================================
179
+ -- §6. EXECUTED 2026-09-14 — from the 2026-09-08 discussion (m3, m4, m5, m8, i7).
180
  --
181
+ -- Verified after the run: 4 new columns and 2 new indexes present, and the
182
+ -- ORM reads all of them against live. Harry still needs this in the
183
+ -- migration set so a fresh environment reproduces it.
184
  --
185
  -- Additive only: three columns and one index on `knowledge_entries`. Nothing
186
  -- is dropped and no column changes type, so a pre-migration reader keeps
 
231
  ALTER TABLE knowledge_documents ADD COLUMN IF NOT EXISTS withdrawn_at timestamptz;
232
  CREATE INDEX IF NOT EXISTS idx_knowledge_documents_live
233
  ON knowledge_documents (scope_id, doc_id) WHERE withdrawn_at IS NULL;
234
+
235
+
236
+ -- ============================================================================
237
+ -- §7. NOT RUN ANYWHERE YET — the review system (r1, r2, r3) from the
238
+ -- 2026-09-08 discussion.
239
+ --
240
+ -- Additive only: three columns, no drops, no type changes.
241
+ --
242
+ -- The point of these, in Mas's words: an entry approved without a human
243
+ -- looking at it must still say WHO allowed that to happen. *"Kalau ada apa-apa
244
+ -- jangan sampai kita yang disalahin… ya udah itu by mereka aja."* It is a
245
+ -- risk-mitigation requirement, not bookkeeping — which is why it belongs in
246
+ -- columns rather than in a free-text note.
247
+ -- ============================================================================
248
+
249
+ -- r1. Which review mode a batch was ingested under. Chosen by the caller at
250
+ -- ingest time: 'expert' leaves entries waiting for a human; 'llm' takes
251
+ -- them straight to approved with no reviewer step at all.
252
+ -- Nullable so every job written before this column existed stays legal.
253
+ ALTER TABLE knowledge_jobs ADD COLUMN IF NOT EXISTS review_mode text
254
+ CHECK (review_mode IS NULL OR review_mode IN ('expert','llm'));
255
+
256
+ -- r2. WHO decided. 'human' is the existing behaviour and the default, so every
257
+ -- review already recorded keeps its meaning without a backfill.
258
+ ALTER TABLE knowledge_reviews ADD COLUMN IF NOT EXISTS decided_by text
259
+ NOT NULL DEFAULT 'human'
260
+ CHECK (decided_by IN ('human','llm'));
261
+
262
+ -- r3. On WHOSE AUTHORITY. The admin who clicked ingest and the expert who
263
+ -- reviews can be different people, and in 'llm' mode there is no reviewer
264
+ -- at all — so the clicker's id has to reach this row or the decision has
265
+ -- no owner. `reviewer_id` stays "who performed the review";
266
+ -- `authorised_by` is "who permitted it to happen this way".
267
+ -- Together they render: approved by LLM with permission of <person>.
268
+ ALTER TABLE knowledge_reviews ADD COLUMN IF NOT EXISTS authorised_by text;
src/knowledge_extraction/mutation.py CHANGED
@@ -83,6 +83,23 @@ def ruled_content(kind: str, payload: Mapping[str, Any] | None) -> dict[str, Any
83
  }
84
 
85
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
86
  def has_mutated(
87
  kind: str,
88
  reviewed: Mapping[str, Any] | None,
 
83
  }
84
 
85
 
86
+ def changed_fields(
87
+ kind: str,
88
+ reviewed: Mapping[str, Any] | None,
89
+ current: Mapping[str, Any] | None,
90
+ ) -> list[str]:
91
+ """Which ruled-on fields moved — the before/after a reviewer is shown (r4).
92
+
93
+ Same comparison as `has_mutated`, reported field by field so the queue can
94
+ say *what* changed instead of making the expert re-read the whole entry to
95
+ find out. That was the ask: show before / after, do not send them back to
96
+ the start.
97
+ """
98
+ before = ruled_content(kind, reviewed)
99
+ after = ruled_content(kind, current)
100
+ return [field for field in before if before[field] != after.get(field)]
101
+
102
+
103
  def has_mutated(
104
  kind: str,
105
  reviewed: Mapping[str, Any] | None,
src/knowledge_ingest/knowledge_store.py CHANGED
@@ -555,6 +555,58 @@ class KnowledgeStore:
555
  index.setdefault(row.label, row.payload)
556
  return index
557
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
558
  async def withdraw_document(self, scope_id: str, doc_id: str) -> int:
559
  """Mark every version of a document withdrawn (i7). Returns how many.
560
 
 
555
  index.setdefault(row.label, row.payload)
556
  return index
557
 
558
+ async def review_diff(
559
+ self, scope_id: str, entity_id: str, kind: str = "glossary"
560
+ ) -> dict | None:
561
+ """Before / after for one entry, for the review screen (r4).
562
+
563
+ Returns None when there is nothing to compare — no decision on record,
564
+ or the entry is gone. `changed` is empty when the approval still
565
+ stands, which is the common case and the one worth making cheap: the
566
+ screen can say "nothing to re-read" instead of showing two identical
567
+ panes.
568
+
569
+ `before` is what the expert had in front of them; for an `edited`
570
+ decision that is their own corrected text, not the model's claim.
571
+ """
572
+ reviews = await self.latest_reviews(scope_id, [entity_id])
573
+ review = reviews.get(entity_id)
574
+ if review is None:
575
+ return None
576
+
577
+ rows = await self.list_entries(scope_id=scope_id, kind=kind, limit=1000)
578
+ current = next(
579
+ (r for r in sorted(rows, key=lambda r: r.created_at or 0, reverse=True)
580
+ if r.entity_id == entity_id),
581
+ None,
582
+ )
583
+ if current is None:
584
+ return None
585
+
586
+ if review.decision == "edited" and review.edited_payload:
587
+ before = review.edited_payload
588
+ else:
589
+ async with AsyncSessionLocal() as session:
590
+ result = await session.execute(
591
+ select(KnowledgeEntryRow).where(
592
+ KnowledgeEntryRow.id == review.entry_id
593
+ )
594
+ )
595
+ reviewed_row = result.scalars().first()
596
+ before = reviewed_row.payload if reviewed_row else None
597
+
598
+ changed = mutation.changed_fields(kind, before, current.payload)
599
+ return {
600
+ "entity_id": entity_id,
601
+ "kind": kind,
602
+ "decision": review.decision,
603
+ "decided_at": review.created_at,
604
+ "changed": changed,
605
+ "status": "waiting_approval" if changed else review.decision,
606
+ "before": mutation.ruled_content(kind, before),
607
+ "after": mutation.ruled_content(kind, current.payload),
608
+ }
609
+
610
  async def withdraw_document(self, scope_id: str, doc_id: str) -> int:
611
  """Mark every version of a document withdrawn (i7). Returns how many.
612