[DamieMoon]

Fixes #61

What

POST /api/ingest in block mode (no source_id) now runs
store.ClassifyBlockAfterUpsert after its UpsertBlock, with the caller's
tenant-resolved block-type set (SnapshotForRequest), on the block's title
and metadata — the same hook POST /api/store, MCP store and the update paths
run. NewIngestHandler takes the block-type registry; server.go passes the
same blocktypeReg the store handler gets.

Why

Block-mode ingest wrote through UpsertBlock and returned without the hook,
so an ingested block kept the row default (knowledge, auto) even when its
title or metadata matched a registry classify rule. The shipped ctx ingest
client sends no source_id, so all of its writes took this path.

Behaviour kept as on every other hook site: type_source='manual' rows keep
their type, derived types are not granted (I7), and a failed classify is
logged at WARN while the chunk still reports updated and the block keeps
the type it already has. Chunk mode (InsertChunk, lifecycle_state='chunk')
is unchanged and not classified. A re-ingest of identical content is a hash
no-op, so blocks ingested before this fix are not reclassified by it (no
backfill).

Docs: the /api/ingest row in docs/api.md, the classify-site list in
docs/architecture.md and the ClassifyBlockAfterUpsert comment (now all nine
call sites from grep -rn 'ClassifyBlockAfterUpsert(' --include='*.go').

Tests

• internal/handler TestIngestBlockModeClassifies: title rule →
  (audit-trail, auto); metadata-only rule {"is_meta": true} →
  (system-meta, auto); manual type kept; chunk mode stays (knowledge, auto);
  no rule keeps the default; derived type not granted; classify failure keeps
  the write (success: true, updated).
• cmd/ctxd TestIngestMountClassifies_Integration through NewRouter:
  a _global rule and a rule only the key's own tenant scope has
  (covers the server.go wiring and SnapshotForRequest).
  Both probes assert type_name and type_source; type_source alone cannot
  show the bug.

Red proof

Each mutation of the fix alone (handler probe / mount probe exit):

mutant	handler	mount
hook call removed (base behaviour)	1	1
nil set at the call	1	1
nil registry in server.go	0	1
nil metadata at the call	1	0
Snapshot() instead of SnapshotForRequest	0	1
hook also after InsertChunk	1	0
classify error fails the chunk	1	0
On base 7f17de2 the mount probe compiles and fails (both subtests read
(knowledge, auto)); the handler probe does not compile there (new
NewIngestHandler parameter).

Gates (go1.27.3)

• go build ./..., go vet ./..., go vet -tags=integration ./...: exit 0
• gofmt -l on changed files: empty
• golangci-lint run --timeout=10m (v2.15.0): 0 issues
• go test -short -race ./...: 65/65 packages ok
• go test -tags=integration -count=1 -p 1 ./internal/handler/... ./cmd/ctxd/...: exit 0
• bash deadcode.sh, bash deadcode.sh testonly: ok
• CGO_ENABLED=0 build + vet for windows/amd64 and darwin/arm64: exit 0

Out of scope (follow-ups)

• StoreHandler.classifySet has the same untested Snapshot() vs
  SnapshotForRequest gap on /api/store.
• Tenant block types are written at scope = ar.TenantID
  (types_write.go typeWriteScope) but resolved by ar.HomeScope
  (RequestTenantScope → buildTenantSet), so API-created tenant types are
  not seen at request time.

🤖 Generated with Claude Code

https://claude.ai/code/session_01S3bwgEeYiQ8cXWgF7FN9J1

Summary by CodeRabbit

• New Features

  • Block-mode ingestion now automatically classifies blocks using their titles and metadata. Manually assigned types remain unchanged; chunk-mode ingestion retains its existing type behavior.
  • Classification issues do not prevent an otherwise successful ingest.

• Documentation

  • Updated API and architecture documentation to describe block-type classification across supported write paths.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Block-mode /api/ingest now attempts automatic type classification after a successful upsert. The handler receives a block-type registry. Integration tests cover classification behavior, and the API and architecture documentation describe the paths.

Changes: Block-mode ingest classification

Layer / File(s)	Summary
Registry wiring and classification flow go/internal/handler/ingest.go, go/cmd/ctxd/server.go, go/internal/handler/derived_write_lock*_integration_test.go, go/internal/store/classify.go, docs/api.md, docs/architecture.md	The handler uses a registry snapshot to classify block-mode upserts. Classification errors are logged without changing the successful write result. Chunk-mode inserts do not enter the classification path. The router supplies the registry, and the documentation describes the classification behavior and paths.
Classification integration coverage go/internal/handler/ingest_classify_integration_test.go, go/internal/handler/ingest_scope_integration_test.go, go/cmd/ctxd/ingest_classify_integration_test.go	Integration tests cover title and metadata rules, tenant rules, manual types, chunk-mode writes, unmatched rules, derived types, classification failures, and the production router.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

• Issue 61: Block-mode /api/ingest now calls ClassifyBlockAfterUpsert and tests the resulting type source.

Suggested reviewers: gottz

Merge Risk: Minimal · up to c94ee

Block-mode ingest gains classification without changing identical-content re-ingests or chunk-mode writes. No merge-blocking issue was identified.

Security architecture risk: Low · up to c94ee

The change reuses existing classification rules without widening the inspected write permissions or tenant scope. Manual type choices and protected derived types remain guarded. Residual risk concerns classification being best-effort rather than atomic with the write.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The inspected path requires an authenticated caller and writes using the caller's home scope, with block identity determined by category, title, and scope. The new classifier receives the ID returned by that upsert rather than a client-selected classification target.

Security Findings and Attack Paths

• observed — The concrete title-to-derived-type authority path is blocked: the classifier refuses derived names before issuing SQL. It also predicates its UPDATE on type_source='auto', protecting manual assignments even when classification follows a concurrent manual change.

Trust Boundaries and Controls

• observed — Caller-controlled title and metadata feed rule matching, but tenant policy selection comes from authenticated request context. Production wiring installs the request-scope resolver, and the ingest handler uses SnapshotForRequest rather than accepting a policy scope from the body.

Resilience and Maintainability Implications

• inferred — Classification failure preserves write availability but does not guarantee the intended type policy has taken effect. The inspected manual and derived authority controls remain effective independently; automatic classification should not be treated as a transactional security barrier.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: block-mode ingest writes now classify like /api/store.
Linked Issues check		Issue #61 requires classification after block-mode UpsertBlock, or documentation of intentional non-classification, plus a test for type_source. processChunk now calls `store.ClassifyBlockAfterU…
Out of Scope Changes check		The changed files support Issue #61. The handler and server changes implement and wire classification. The handler and mount tests verify the behavior and request-scoped registry wiring. The documenta…

Full details: Linked Issues check

Explanation

Issue #61 requires classification after block-mode UpsertBlock, or documentation of intentional non-classification, plus a test for type_source. processChunk now calls store.ClassifyBlockAfterUpsert after a successful block-mode upsert. It passes h.classifySet(ctx), block.ID, block.Title, and block.Metadata. classifySet uses SnapshotForRequest. The handler logs classification errors and keeps the write result. Chunk mode does not call the hook. server.go passes the shared blocktypeReg to NewIngestHandler. The added handler and mount integration tests assert classified type_name and type_source, including manual-type preservation and chunk-mode behavior.

Full details: Out of Scope Changes check

Explanation

The changed files support Issue #61. The handler and server changes implement and wire classification. The handler and mount tests verify the behavior and request-scoped registry wiring. The documentation and classifier comment describe the new ingest behavior and call site. Constructor updates keep existing tests and helpers valid. No demonstrated unrelated implementation change is present.

Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 7 functions across 8 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 79af832a-7005-4e0e-a91f-ab0dfdc44f54

Commits

Reviewing files that changed from the base of the PR and between 7f17de2 and c94ee7a.

Files selected for processing: 10

• docs/api.md
• docs/architecture.md
• go/cmd/ctxd/ingest_classify_integration_test.go
• go/cmd/ctxd/server.go
• go/internal/handler/derived_write_lock_integration_test.go
• go/internal/handler/derived_write_lock_r2_integration_test.go
• go/internal/handler/ingest.go
• go/internal/handler/ingest_classify_integration_test.go
• go/internal/handler/ingest_scope_integration_test.go
• go/internal/store/classify.go

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

[GottZ]

Review — fix(ingest): classify block-mode ingest writes like /api/store (#69)

Reviewed: root @ 7f17de21 → fix/61-ingest-classify @ c94ee7a4 (1 commit, 10 files, +410/−16). Merge base is the current root tip.
Executed: in a worktree at c94ee7a4: go build ./..., go vet -tags=integration ./..., golangci-lint run with and without --build-tags=integration (0 issues each), go test -tags=integration -p 1 -run 'TestIngest|TestDerivedWriteLock' ./internal/handler/ ./cmd/ctxd/ (ok; the 7 + 2 new subtests all ran, none skipped). Red-proof via go test -overlay without the hook call: handler probe red at ingest_classify_integration_test.go:97 (("knowledge","auto"), want (audit-trail, auto)) and :116, mount probe red at :134/:139; with the base server.go (nil registry) the mount probe is red as well. Mutation sweep over 7 further variants (details on request).
Read: handler/ingest.go end to end, store/classify.go, blocktype/registry.go (snapshot path), blocktype/builtin.go (rules), cli/ingest.go (payload), all NewIngestHandler callers, the other eight ClassifyBlockAfterUpsert sites, both docs hunks, migrations/113_baseline.sql:700-706.
Body vs head: matches.

1. What the change does

Block-mode POST /api/ingest now runs store.ClassifyBlockAfterUpsert after UpsertBlock with the caller's tenant-resolved set (SnapshotForRequest), argument-for-argument the same call as /api/store (context_store.go:196), MCP store and the confirm path; NewIngestHandler takes the registry and server.go passes the one the store handler gets. Chunk mode stays unclassified. A failed classify is logged at WARN and the chunk still reports updated. The doc comment on the hook and docs/architecture.md now list all nine call sites (my own grep agrees: nine non-test sites). Diff and body agree.

2. Overlap and duplicates

Search scope: gh search prs on "ingest classify" (open and closed), "ClassifyBlockAfterUpsert", "#61"; file-path intersection with open #55/#57/#63/#67.

PR	Status	Class	Consequence
#55	open	related, distinct (no shared files)	docs/ctx-as-agent-memory.md:14 (at 6dc5215b) lists where the classifier runs and does not name ingest; once this lands that sentence is one site short — worth a word in #55 (same author).
#63	open	textual neighbour only	no shared files; no ordering constraint.

No open or closed PR covers this fix.

3. Findings (ranked)

Low — the registry snapshot is resolved per chunk, not per request [read]

• Category: cost
• Where: go/internal/handler/ingest.go:322 → :357-367; miss path go/internal/blocktype/registry.go:175-206
• Entry point: POST /api/ingest with up to maxIngestChunks = 200 block-mode chunks.
• Before → after: on a cache hit SnapshotForRequest is one atomic load, so the steady state costs nothing measurable. On the miss path (tenant set build fails) the registry caches nothing and logs a WARN per call, so one request becomes up to 200 rebuild attempts and 200 WARN lines where /api/store produces one. A reload landing mid-batch also classifies the chunks of one request against two rule generations.
• Who pays: a tenant whose type overlay fails to load; frequency rare; magnitude not measured.
• Fix: resolve set := h.classifySet(ctx) once in HandleIngest before the chunk loop and pass it into processChunk (the store handler resolves once per request too). Fine as a follow-up.

Low — the docs sentence about chunk rows is one edge short [read]

• Category: claims
• Where: docs/api.md:46 ("A chunk-mode row is not classified; it keeps the type it has") vs. go/migrations/113_baseline.sql:704-705
• Entry point: a block-mode write (no source_id) whose (category, title, scope) equals an existing InsertChunk row.
• Before → after: the unique index has no lifecycle filter, so the block-mode upsert lands on the chunk row and the hook classifies it. This was already reachable through /api/store before this PR; what is new is the sentence that promises chunk rows stay unclassified.
• Fix: "Chunk-mode writes do not run the classifier" (a statement about the write path, not about the row).

Low — every probe sends a single chunk [executed]

• Category: tests
• Where: handler probe :66, mount probe :97
• Evidence: a mutation that classifies only index == 0 passes all nine subtests.
• Fix: one request with two or three block chunks (and one mixed with a chunk-mode chunk), asserting the type of each — pins the loop, not just the call.

4. Declared behaviour changes (not defects)

• ctx ingest always writes in block mode (cli/ingest.go:204-211 sends no source_id), so every vault note now goes through the title/metadata classifier. With the built-in rules a note whose title contains session, performance, reset, baseline, … becomes audit-trail (damped in retrieval); a title starting with "Compaction source" / "Compaction checkpoint head" becomes checkpoint (retrieval-excluded). That is the point of #61 and the same policy /api/store applies; stating it here so nobody reads it as a regression later.
• No backfill: identical content is a hash no-op before the hook (ingest.go:262-264), as on /api/store. Blocks ingested before this fix keep their type until a content change or the classify job.

5. Checked and found in order

Hook arguments identical to context_store.go:196, mcp.go:642, confirm_core.go:247 (metadata from the store return) · manual type kept: the UPDATE carries AND type_source = 'auto' (classify.go:90-93) and the upsert without a type does not touch type_source · nil metadata becomes {} (blocks.go:534-536) · derived types declined by title (I7, classify.go:84-88), so no new I7 door · S3/provenance refusal runs inside the upsert transaction before the hook; the chunk ends before the hook on every error arm · tenant resolution uses the key's home scope, the same scope the write goes to · all seven NewIngestHandler callers adjusted · the only auto-typed writer left without the hook is InsertChunk, declared in code and docs · helpers (ingestKey, postIngest, firstResult, seedSource, wlCatalogTitle, applyEnv, loadFromEnv) exist · probes write through the production path (HandleIngest, NewRouter) · "type_source alone cannot show the bug" holds (the hook writes only type_name) · CI runs both new tests (handler shard ci.yml:654-655, cmd/ctxd in the rest complement) · one signed commit, author = committer, conventional title, base = current root tip.

6. Existing review coverage

• CodeRabbit walkthrough (2026-11-03T01:18Z): no actionable comments.
• No human review before this one. All three findings and the two declared changes are new here.

7. Verdict

Approve. The fix is the /api/store hook at the one write path that lacked it, the red-proof is executed, nothing is above Low. The three Lows (per-request snapshot, docs wording, multi-chunk probe) and the #55 sentence are follow-ups, not merge conditions.

1. Hoist classifySet before the chunk loop.
2. Reword the chunk-row sentence in docs/api.md; add ingest to the classify-site list in #55.
3. Add a multi-chunk probe.

8. Not verified / questions for the author

• Cost of the hook UPDATE itself (not HOT, seven indexes on type_name, two triggers) is a property of the existing hook on all nine sites, not measured here.
• [hypothesis] the mount probe seeds its tenant rule by raw SQL at the home scope; whether the type-* manage API produces a row the same resolution path sees was not checked (the body calls it out of scope).