Skip to content

Repo Context Audit: add governed model running summaries#1803

Closed
Niko4417 wants to merge 13 commits into
devfrom
issue/1765-model-running-summary-clean
Closed

Repo Context Audit: add governed model running summaries#1803
Niko4417 wants to merge 13 commits into
devfrom
issue/1765-model-running-summary-clean

Conversation

@Niko4417

@Niko4417 Niko4417 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a governed Model Gateway-backed structured running-summary path for compacted chat history so long sessions preserve ordinary decisions, constraints, files/symbols, debugging context, and open threads without ungoverned model calls or user-role injection.

Resolves #1765

Scope

  • In scope: structured summary contracts and validation, Model Gateway JSON response-format wiring, deterministic invalid/unavailable/timeout failure metadata, system-scoped resurfacing, evidence redaction, and regression tests.
  • Out of scope: repository-understanding retrieval/indexing, graph traversal, cross-language tracing, provider tokenizer calibration, UI changes, and direct provider calls outside the Model Gateway.

Reuse And No-Duplication

  • Existing Keiko functionality was inspected before implementation.
  • This PR reuses, extends, generalizes, or consolidates existing functionality where practical.
  • Any new implementation is limited to a documented capability gap in the linked issue.
  • This PR does not introduce a parallel workspace, graph, relationship, policy, evidence, memory, connector, workflow, or UI subsystem where an existing subsystem can be extended.
  • Refactoring or consolidation was considered when existing functionality was close but not shaped for this change.

Delivery Board

  • Linked issue is in the public Keiko Product Delivery project.
  • Linked issue has a valid Parent Epic: #1720 unless this PR updates an epic container directly.
  • Linked issue is attached as a GitHub sub-issue of its parent epic so it appears in the correct board swimlane.
  • Parent epic is on the board with Classification: Epic, Status: Open Epics, and priority/order set for top-to-bottom implementation sequencing.
  • Linked task/card issue is on the board with Classification: Task, Status: In Progress while active, inherited or explicit Priority, and Human Review Required: Yes.
  • Project Status is In Progress while work is active, or Done only after merge and closure evidence.
  • Project Workflow State is PR Open or Ready for Human Review.
  • Owner / Agent, Branch, Pull Request, and Human Review Required are filled.
  • Issue label reflects the current state: status: in progress, status: ready for human review, or status: done after merge.
  • Autonomous agents did not merge into dev, enable auto-merge, close the issue, or bypass human review unless explicitly authorized by the human maintainer.

Product Impact

  • UI or user workflow
  • CLI or developer workflow
  • Core generation engine
  • Evidence, audit, or compliance artifact
  • Security or supply chain
  • Packaging, release, or npm publication
  • Documentation or repository hygiene
  • No user-facing behavior change

Update Impact

  • Not release-impacting.
  • Release-impact catalog or issue metadata updated.
  • Release-note category: improvements.
  • Priority: high.
  • User-visible change: Long compacted chat sessions preserve ordinary unlabeled decisions, constraints, debugging context, and open threads more faithfully under model context pressure.
  • Release-note bullet: Improves long-session compaction by using governed model-written structured running summaries with auditable attribution.
  • Supported-from versions: next release after merge to dev.
  • Affected state stores: compaction evidence manifests may include validated generated-summary metadata; legacy summaries remain accepted when content is present and no structured fields are used.
  • User action required and remediation: none expected.
  • Internal-only rationale, if applicable: not internal-only.
  • Release-owner review evidence: npm run check:release-impact passed locally.

Verification

Required:

  • Required GitHub checks pass before merge.
  • Local verification commands or rationale are listed below.
  • Reuse/extension/generalization evidence or gap rationale is listed below.

Local verification:

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH .keiko-scripts/verify-receipt.sh 1765
# GREEN at issue/1765-model-running-summary-clean @ b72f2419
# verify.sh: 970 test files passed, 16,364 tests passed, 1 skipped

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH .keiko-scripts/audit-receipt.sh 1765 --findings 0 --user-facing false
# audit receipt written at issue/1765-model-running-summary-clean @ b72f2419 (findings=0, user_facing=false)

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npx vitest run packages/keiko-contracts/src/text-safety.test.ts packages/keiko-contracts/src/context-engineering-compaction.test.ts packages/keiko-contracts/src/local-knowledge-pods.test.ts packages/keiko-evidence/src/compaction-evidence.test.ts packages/keiko-evidence/src/qualityIntelligence/__tests__/verificationCache.test.ts packages/keiko-evidence/src/listByPrefix.test.ts packages/keiko-local-knowledge/src/knowledge-pods.test.ts packages/keiko-local-knowledge/src/index.test.ts packages/keiko-server/src/chat-compaction-model-summary.test.ts packages/keiko-server/src/chat-compaction-resurfacing.test.ts packages/keiko-server/src/chat-compaction-resurfacing.scale.test.ts packages/keiko-server/src/chat-prompt-budget.test.ts packages/keiko-server/src/chat-gateway-assembly.test.ts packages/keiko-server/src/chat-compaction-evidence.test.ts packages/keiko-server/src/conversation-compaction.test.ts packages/keiko-server/src/conversation-structured-compaction.test.ts packages/keiko-server/src/chat-stream-handlers.test.ts packages/keiko-server/src/desktop-chat-handlers.test.ts packages/keiko-server/src/local-knowledge-handlers.test.ts packages/keiko-server/src/qualityIntelligence/__tests__/reviewStore.locking.test.ts
# PASS: 20 files, 328 tests

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npx vitest run tests/scripts/dev-runner-readiness.test.ts packages/keiko-cli/src/verify.test.ts packages/keiko-evidence/src/listByPrefix.test.ts
# PASS: 3 files, 19 tests

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npx vitest run packages/keiko-evidence/src/qualityIntelligence/__tests__/retentionEnforcement.test.ts
# PASS: 1 file, 17 tests

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npx eslint packages/keiko-contracts/src/text-safety.ts packages/keiko-contracts/src/text-safety.test.ts packages/keiko-cli/src/verify.test.ts packages/keiko-evidence/src/listByPrefix.test.ts packages/keiko-evidence/src/qualityIntelligence/__tests__/retentionEnforcement.test.ts tests/scripts/dev-runner-readiness.test.ts --max-warnings=0
# PASS

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run test:coverage:packages
# PASS: 976 test files, 16,445 tests passed, 1 skipped

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run check:coverage:branches
# PASS: all selected package branch floors satisfied; keiko-evidence branches=78.24%, floor=78.24%

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run check:context-quality
# PASS: context-quality PR1-PR4 metrics green; chat-p95=15.42ms

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run check:release-impact
# release-impact: PASS - current package version has reviewed update-impact metadata

PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run clean && PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run build && PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run build:ui && PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run prepare:bin && PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run prune:package-native-optionals && PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run prune:package-build-artifacts && PATH=/Users/nikolaos.vasilopoulos/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm run check:package-surface
# package-surface check passed: 4159 files, dist/ui/static present

# Bot review comments considered:
# - github-advanced-security CodeQL regex thread was already resolved/outdated after cd4477f7.
# - github-code-quality off-by-one scanner thread addressed by 1a6c8ad0 and covered by text-safety regression tests.

Reuse / gap rationale:

The implementation extends existing context-engineering compaction contracts, compaction evidence redaction, chat prompt-budget assembly, and chat resurfacing paths. Model-written summaries stay behind the existing ModelPort/Model Gateway boundary and use explicit responseFormat schema validation instead of a parallel provider path. The remaining gap was structured, governed extraction of ordinary unlabeled decisions, constraints, files/symbols, debugging context, and open threads after compaction; deterministic fallback behavior is preserved for unavailable, timed-out, invalid, unsafe, or legacy summaries.

Select only what applies:

  • UI behavior manually verified or covered by tests.
  • CLI behavior verified with command output or tests.
  • Core logic covered by unit, integration, property, or fixture tests.
  • Security-sensitive change reviewed for trust boundaries, secrets, external calls, and generated artifacts.
  • Supply-chain or package-surface change verified with package, license, lockfile, SBOM, or npm dry-run checks.
  • Documentation or Markdown change verified by the repository link check or a targeted local equivalent.
  • Release-impacting change verified with npm run check:release-impact or an explicit rationale.
  • Not applicable items are explained below.

Not applicable rationale:

  • No UI surface is changed.
  • No CLI command behavior is changed.
  • No Markdown documentation files are changed.

Review And Closure

  • The PR implements only the linked issue scope.
  • Actionable review findings are fixed or explicitly dispositioned.
  • Unresolved review threads are resolved before merge.
  • Checks are repeated after the latest pushed fix.
  • Issue acceptance criteria and closure evidence are updated only where evidence exists.
  • The linked issue records reuse, extension, generalization, or new-gap rationale before closure.
  • Delivery board status is updated before requesting final maintainer review.
  • Use Resolves #<issue_number> only when this PR should close the issue.

Risk Notes

The generated summary is model-authored and therefore treated as untrusted input: it is schema-validated, bounded, redacted, persisted with validation/failure state, and resurfaced only through system-scoped compaction context. Legacy summary manifests remain accepted for compatibility when they have content and no structured metadata. The PR intentionally does not merge to dev, close the issue manually, or bypass human review.

@Niko4417 Niko4417 requested a review from oscharko as a code owner July 2, 2026 14:52
@Niko4417

Niko4417 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Audit review — PR #1803 (Resolves #1765)

Verdict: Approve with minor non-blocking nits. All four acceptance criteria are met with regression coverage, security posture is preserved, and CI (ci + ui) is green at e6788397.

Acceptance criteria — verified

  • Structured fields survive compactiondecisions / constraints / filesAndSymbols / debuggingContext / openThreads are parsed, sanitized, bounded, persisted, and resurfaced (chat-compaction-model-summary.ts, chat-compaction-resurfacing.ts). ✅
  • Generated summary text is never role: "user"chat-prompt-budget.ts now routes compaction context through insertSystemScopedCompactionContext (system-scoped) instead of folding it into the latest user turn; asserted by chat-gateway-assembly.test.ts ("keeps included resurfaced compaction context system-scoped"). ✅
  • Invalid / unsafe / oversized / unavailable cannot corrupt the prompt — explicit invalid | timed-out | unavailable statuses persist content: "" + validationState: "rejected" + a bounded failureReason; four failure-mode tests cover invalid structured output, model-unavailable, timeout (fake timers), and legacy-text fallback. ✅
  • Evidence preserves attribution + validation state — schema/validation extended in context-engineering-compaction-validation.ts; redaction extended in compaction-evidence.ts; rejected summaries are excluded from resurfacing via isRejectedModelSummary. ✅

Strengths

  • Untrusted-output discipline is thorough: schema-constrained (json_schema, strict), per-item + array bounds, NFKC + format-char stripping, code-fence rejection, global absolute-path redaction (fixes the prior first-match-only String.replace), and secret redaction — followed by a defense-in-depth ABSOLUTE_PATH_PATTERN.test reject.
  • Backward compatibility preserved: legacy summaries (no status) still validate and resurface; structured fields are gated on modern metadata being present.

Non-blocking findings

  1. validationState: "accepted" is effectively unreachablenormalizeSummaryText sets changed: clamped !== raw, comparing the fully normalized/trimmed value against the raw model string. Any trailing whitespace, CRLF, or NFKC difference flips changed, so nearly every summary persists as "redacted" even when nothing was removed for safety. This dilutes the audit signal (redacted should mean content was actually stripped). Suggest tracking redaction explicitly (e.g. compare post-redaction vs pre-redaction, or a dedicated flag) rather than post-normalization vs raw. — packages/keiko-server/src/chat-compaction-model-summary.ts normalizeSummaryText

  2. Dead fallback branch in parseStructuredSummarysanitizeSummaryContent already returns undefined (→ rejected) for empty content, so by the time summaryContentFromFields(content.value, ...) runs, content.value is always non-empty; the synthesizeSummaryContent branch and its subsequent === undefined → "unsafe-output" guard are unreachable via this path. Harmless, but worth simplifying or adding a test that actually exercises synthesis. — same file

  3. Residual trust-elevation risk (per-spec, tracking only) — the validated summary is now injected as a system message, raising its apparent authority vs. the prior user-turn placement. Redaction strips secrets/paths/code-fences but not imperative injected instructions. This is exactly what Repo Context Audit: add Model Gateway-backed structured running summaries #1765 mandates (never role:user; system/developer-scoped) and content is schema-bounded, so it is an accepted tradeoff — noting it as residual risk to keep on the radar, not a change request.

None of the above block merge. The PR implements only the linked issue scope and preserves deterministic-fallback semantics.

…ld synthesis (#1765)

Address two confirmed review findings on PR #1803:

- validationState "accepted" was effectively unreachable: normalizeSummaryText
  derived `changed` from cosmetic normalization (NFKC, CRLF, whitespace collapse,
  trim), so nearly every summary persisted as "redacted". `changed` now reflects
  only safety-relevant transforms (secret/absolute-path redaction fired, unsafe
  format chars stripped, or content clamped to the max-chars bound). Cosmetic
  normalization alone no longer marks a summary as redacted.

- Blank model content discarded valid structured fields: sanitizeSummaryContent
  returned undefined when content normalized to empty, causing parseStructuredSummary
  to reject the whole summary and leaving synthesizeSummaryContent(fields) as dead
  code. It now returns an empty value for unusable-but-string content so content is
  synthesized from the already-sanitized structured fields (each item independently
  redacted), preserving decisions/constraints/files/debugging/open-threads.

Adds two regression tests. Incidental hunks are the project format-on-save hook
normalizing pre-existing non-prettier-clean lines; no behavior change.

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

Niko4417 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Review findings addressed — a6da6732

Both actionable findings from the prior review are fixed (finding 3 was per-spec residual risk, no change). Scope confined to packages/keiko-server/src/chat-compaction-model-summary.ts + its test file.

Finding 1 — validationState: "accepted" was effectively unreachable
normalizeSummaryText derived changed from clamped !== raw, i.e. cosmetic normalization (NFKC, CRLF→LF, whitespace collapse, trim) flipped it, so nearly every summary persisted as "redacted". changed now reflects only safety-relevant transforms: secret/absolute-path redaction fired (redacted !== raw), unsafe format chars stripped (stripUnsafeFormatChars(redacted) !== redacted), or content clamped to the max-chars bound (clamped !== compacted). Cosmetic normalization alone no longer marks a summary redacted.

Finding 2 — blank model content discarded valid structured fields (dead synthesis branch)
sanitizeSummaryContent returned undefined when content normalized to empty, so parseStructuredSummary rejected the whole summary and synthesizeSummaryContent(fields) was unreachable. It now returns an empty value for unusable-but-string content, so content is synthesized from the already-sanitized structured fields (each item independently secret/path-redacted via sanitizeSummaryItems), preserving decisions/constraints/files/debugging/open-threads instead of dropping them. Security invariant verified: synthesized content is built solely from sanitized fields — no raw/unredacted text can reach it.

Regression tests added (chat-compaction-model-summary.test.ts):

  • accepts a structured summary needing no safety redaction despite cosmetic whitespace → asserts validationState: "accepted" (fails under the old clamped !== raw logic).
  • synthesizes content from structured fields when the model returns blank content → exercises the formerly-dead synthesis path; asserts non-empty synthesized content, secret redaction, and preserved fields.

Local verification: typecheck, lint, check:version-consistency green; the three affected suites pass (chat-compaction-model-summary, chat-compaction-resurfacing, context-engineering-compaction — 78 tests). arch:check and the full UI test step can't run under local Node 25 (dependency-cruiser/jsdom limitation); GitHub CI (Node 22) is the authoritative gate and is running on this push.

Incidental non-logic hunks in the source file are the repository's format-on-save hook normalizing pre-existing non-prettier-clean lines (prettier is not a CI gate; behavior unchanged).

Comment thread packages/keiko-contracts/src/text-safety.ts Fixed
Comment thread packages/keiko-contracts/src/text-safety.ts Fixed
@Niko4417

Niko4417 commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #1907, which combines #1765 with #1766 and #1809 into one reviewed branch and carries the consolidated audit evidence.

@Niko4417 Niko4417 closed this Jul 5, 2026
oscharko added a commit that referenced this pull request Jul 5, 2026
…1907)

* feat(context): add governed model running summaries

Refs #1765

* feat(context): add calibrated token accounting

* fix(context): tighten model-summary validationState and reachable field synthesis (#1765)

Address two confirmed review findings on PR #1803:

- validationState "accepted" was effectively unreachable: normalizeSummaryText
  derived `changed` from cosmetic normalization (NFKC, CRLF, whitespace collapse,
  trim), so nearly every summary persisted as "redacted". `changed` now reflects
  only safety-relevant transforms (secret/absolute-path redaction fired, unsafe
  format chars stripped, or content clamped to the max-chars bound). Cosmetic
  normalization alone no longer marks a summary as redacted.

- Blank model content discarded valid structured fields: sanitizeSummaryContent
  returned undefined when content normalized to empty, causing parseStructuredSummary
  to reject the whole summary and leaving synthesizeSummaryContent(fields) as dead
  code. It now returns an empty value for unusable-but-string content so content is
  synthesized from the already-sanitized structured fields (each item independently
  redacted), preserving decisions/constraints/files/debugging/open-threads.

Adds two regression tests. Incidental hunks are the project format-on-save hook
normalizing pre-existing non-prettier-clean lines; no behavior change.

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

* fix(context): address PR #1802 audit findings for calibrated token accounting (#1766)

- Split chat-prompt-budget-diagnostics.ts under the 400-LOC cap by extracting
  the token-summary helpers into chat-prompt-budget-token-summary.ts.
- Document the deliberate lane attribution: the compaction summary's tokens are
  charged to the history-summary lane (which owns the +1 summary item), while
  system-contract reports only the bare canonical system prompt.
- Pin the compacted-path lane split with a mutation-robust test.
- Reject stray scaleMilli/offsetTokens under source "fallback-estimated".
- Document the defensive clamp in calibratedTokenCount; add redactTokenAccounting
  and validator-rejection test coverage.

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

* test(qi): stabilize verification cache regression

Refs #1766

* fix(context): harden model summaries after dev merge

Refs #1765

* fix(evidence): prime QI verification cache after writes

Refs #1765

* test(evidence): stabilize performance gate timing

Refs #1765

* fix(gateway): redact token counter ids from browser projections

Refs #1766

* fix(security): make role marker scanning linear

* test(evidence): cover QI verification cache edge cases

* test(evidence): stabilize lock reclaim regressions

Refs #1766

* fix(security): guard pseudo-role scanner bounds

* test: cap root vitest workers for stable receipts

Refs #1766

* test(ci): stabilize full-suite timeout-sensitive checks

* test(evidence): stabilize QI retention count gate

* test: tighten root vitest worker cap

Refs #1766

* fix: harden governed updater UX

* test: stabilize QI verification cache budget

* test(package): update safe capability surface contract (#1766)

* test: relax dev runner readiness timeout

* test: harden evidence prefix cleanup

* test(package): keep safe capability surface sorted (#1766)

* test: relax release-impact notes timeout

* test: format evidence prefix cleanup

* test: cover updater review follow-ups

* test: satisfy updater playwright lint

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Oliver Scharkowski <59687448+oscharko@users.noreply.github.com>
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.

Repo Context Audit: add Model Gateway-backed structured running summaries

2 participants