Repository navigation
Conversation
…on turn; transcripts are read-only legacy (#18293)
…ssion delete clears the LLM-context cache, docs name chat_sessions the record (#18293)
…r does not durably own; admin route 403s; location from SSOT config (#18293)
…cy-only id cannot be taken, and deploy templates pin the location (#18293)
…no chat_sessions turns starts with empty context (#18293)
…ion-turn-store # Conflicts: # repo_tests/reach_scope_claim_17844_test.py
…s, and re-pin three floors mid-window after the tree grew (#18293)
…18293-conversation-turn-store # Conflicts: # repo_tests/collected_modules_inert_on_import_test.py # repo_tests/reach_scope_claim_17844_test.py
…sue-18293-conversation-turn-store # Conflicts: # repo_tests/python_file_size_ratchet_baseline.py # scripts/python_file_size_known_large.py
…er the tree grew (#18293)
…on decisions, pin the writer set (#17225)
…, adapt audit_logger, route SecurityLayer, one retention table (#17225) Owner decision 2026-10-10, D2/D3/D5/D6: - D2: services/audit/audit_log.py, services/event_log.py and knowledge/audit_log.py are deleted; their vocabularies move to services/audit/taxonomy.py and every caller uses the canonical service (voice_bundle_admin and auth via emit_compliance; the knowledge audit API via services/audit/knowledge_view.py, which ends the double write). Old keys are left to expire by their TTL; no stored data is touched. - D3: services/audit_logger.py is an adapter: an AuditEntry is recorded as a SECURITY AuditEvent and read back from the canonical trail. - D5: services/audit/retention.py is the one retention table; projection TTLs and the one purge path (services/audit/purge.py) read it. The nightly task no longer trims every audit:* zset at one period. - D6: SecurityLayer.audit_log records through emit_security; the JSONL file is no longer written or read. Fixes found on the way: audit.emit was shadowed by an EventType bridge, so emit(AuditEvent(...)) recorded a COMPLIANCE event named after the event's repr (7 producers); a producer on a thread with no loop now hands its write to the serving loop; reads scan newest-first; an unreadable trail raises instead of reading as empty (#13258). The writer guard detects audit ORM rows, INSERT INTO ...audit and file writes in modules binding an audit path, which found five stores the first inventory missed. D1 (which Postgres table) and D4 (daemon key move) are open; see docs/developer/AUDIT_SYSTEMS.md.
…its projection (#17225) Owner decision 2026-10-11 on D1: extend the existing Postgres audit_logs table rather than add audit_event. - Additive schema: category (NOT NULL, server default 'security'), actor, session_id, resource_ref and two indexes. Alembic 20261011_098 on the backend; add_audit_logs_canonical_columns on the SLM's slm_users database, which maps the same shared model. No existing row is rewritten; the pre-existing twelve columns are pinned unchanged. - One row constructor, autobot_shared/user_management/audit_rows.audit_row, used by the canonical service and by the shared RBAC denial, organisation and team writers, so both processes write one schema. The backend and SLM user_service.py still construct AuditLog directly; they move onto audit_row after #18267, which rewrites both files. - services/audit/store.py: record() writes the row, then the Redis projection; every query is one SQL statement on audit_logs; rebuild_projection() reconstructs the projection. store_authority declares audit_trail POSTGRES with a REDIS projection. - .secrets.baseline: drop the entries for the two deleted legacy modules.
…ontrol plane map has an audit-trail row (#17225) with audit_rows.audit_row, so every audit_logs row has one constructor. The writer guard drops the two user_service allowances and fails on any AuditLog(...) outside the row builder (the SLM's own node-audit model excepted). - CONTROL_PLANE_MAP.md: row 18, Audit trail, Enforced. - AUDIT_SYSTEMS.md: D1 done; rows 5 and 10-12 point at #18284, #18285 and #18286. - symbol_fork_baseline.json: drop KnowledgeAuditLog, audit_record, emit and query_events, which are no longer fork clusters. - audit_rows_test records the mapping without configuring the shared mapper registry, which needs each backend's concrete Organization.
…ound the orphan lock, lift the request cap off the background scan (#18013) _run_duplicate_analysis signalled 'lock held' and 'deadline expired' with the same None, so every caller reported 'timed out after 120s' - including a request that returned in 0.2s because a scan was already running, with advice (raise min_similarity) that cannot help. They are now ScanBusy (carrying the holder's age) and ScanTimedOut (carrying the deadline that actually elapsed), each with its own payload; report.py logs which. The deferred lock release (#13602) relied on the orphaned executor thread exiting. A thread that never exits left every later call declined for the life of the process. Past AUTOBOT_DUPLICATE_ORPHAN_LEASE_SECONDS after the scan was told to stop, the lock is reclaimed; the late callback releases the lock object it captured, so it can never free one a newer scan owns. The Celery task applied the 120s request deadline, so the queued path that exists to escape it timed out the same way and reported completed. It is now bounded by AUTOBOT_DUPLICATE_BACKGROUND_TIMEOUT_SECONDS and a timeout raises instead of returning a partial result (#18067). duplicates.py leaves its grandfathered ceiling (660 -> 470): result shaping and config-duplicate helpers moved unchanged to duplicate_results.py. Refs #18013, #18067
… failure, and a lookup that threw is not 'no such source' (#17982) Celery records a RETURNED value as SUCCESS whatever it says, so the three scan tasks that return a refusal dict (import_tree, duplicate, dependency, all through _scan_root_or_refusal) surfaced as completed / progress 100 / error None. celery_result_to_status now maps a payload whose own status is 'error' to failed and carries the reason to the status endpoint; this is the one shared mapper, so all three are covered and asserted, not only the duplicate one. The duplicate task also refuses a scan that examined zero files. /duplicates/cached answered HTTP 200 for a failed scan, which is how a consumer checking only the status code rendered it as 'no duplicates'. It is 409 now with a fixed message; the task's exception text is logged and served by the status endpoint, not echoed (CWE-209). resolve_source_root swallowed every lookup failure into logger.debug and returned None, and source_storage.get_source returned None when it could not build its Redis client. A fault and an unknown id were therefore the same observable, and the refusal asserted the id was unknown. Both now raise RegistryUnavailable, logged at error with the cause chained; the default-source lookup no longer falls through to AutoBot's own tree on a registry fault. A structural guard fails if any broad handler around a source lookup swallows instead of re-raising, anywhere in the package - the #2760 extraction left a copy behind because nothing guarded more than one file. Not established: why the source id did not resolve on the deployment that produced the original report. The swallowed exception was never logged above DEBUG, so the cause is unrecoverable; what is fixed is that the next occurrence names its cause. Refs #17982
…wn tree, and config-duplicates says when it read nothing (#18067) Why the 120s timeout and the zero results. Measured on the deployment's own backend log for the registered AutoBot source: 8,377 files -> 130,257 blocks -> an LSH index of ALL 130k blocks -> 3.96M candidate pairs -> 2.54M reported 'duplicates' (2.43M of them 'low'), about 9 minutes end to end. The request deadline (120s) always fired first, so the response was an empty 'partial'. Three causes, each independent: 1. LSH was built at a hardcoded 0.5 whatever min_similarity was, so the advice in every timeout message ('try a higher min_similarity') changed the answer but not the work. 2. Half of the blocks have <= 17 distinct tokens. Token-set Jaccard on sets that small reaches 0.5 by coincidence; they flooded the candidate set. 3. 'No artificial limit on returned pairs' (#609) meant 2.5M pairs were built, held, and returned - no panel, Redis result or JSON response can carry that. Fix, in a new duplicate_lsh.py (duplicate_detector.py was grandfathered at its size ceiling and shrinks 1326 -> 1120): the fuzzy pass takes blocks with >= 20 distinct tokens (exact-hash matching is unchanged and still covers every size); the index is built LSH_RECALL_MARGIN (0.1) below min_similarity because MinHash-LSH retrieves a pair AT the threshold only ~50% of the time; MinHash is one update_batch call per block; the query loop is cancellable. The returned list is capped at AUTOBOT_DUPLICATE_MAX_RETURNED (5000) while total_count and the high/medium/low statistics still describe every pair, and `truncated` says so. Measured on this repository after the change: 8,384 files, 175s, 56,557 pairs found (25,491 high), 5,000 returned, truncated=true - on the background path, which is bounded by AUTOBOT_DUPLICATE_BACKGROUND_TIMEOUT_SECONDS and not the request deadline. The synchronous GET still exceeds 120s on this tree and now says so honestly. Also fixed: detect_duplicates_async had no min_similarity parameter, so the one caller that passed it raised TypeError on every semantic request and silently fell back to the standard scan. config-duplicates: detect_config_duplicates scanned <root>/src and <root>/backend. AutoBot has neither, so it read zero files and answered 'No configuration duplicates found!' in 0.7s. It also skipped any file whose ABSOLUTE path contained 'test'. It now scans from the root when the legacy directories are absent, prunes SKIP_DIRS, judges test files on the relative path, counts files scanned and failed (parse failures were logged at DEBUG), and the endpoint answers 422 when it read nothing. Its report states how many files the zero is about. Known-answer fixtures: a planted duplicate pair that must be found and a near-miss that must not, for the block detector and for config-duplicates. Not done here, still open on #18067: the symbol-fork detector reaching analytics, the per-detector ran/zero/failed record for the cross-file rules, and the problem-type counts. Refs #18067, #18013, #17982
…an unreadable registry on purpose (#17982) get_source and get_default_source_id now raise RegistryUnavailable where they returned None. Audited every caller (backend, LLC, tasks; SLM and scripts have none): authorization answers 503 instead of a false 404, HTTP handlers resolve the default source through shared.default_source_id (503), an app-level handler turns any remaining fault into 503 without echoing the cause, the LLC project list degrades without the source summary, and project disposal aborts rather than orphaning the clone. Celery tasks let it fail the task. Refs #17982
…he memory-graph scorer (#18183)
…e registry reach its 503 handler (#17982)
…em validator exercises the canonical BM25+RRF HybridSearcher (#17599)
…ob cannot delete alice's chat, and one guard for un-awaited ChatHistoryManager coroutines in the API (#18316)
…r indexes concurrently (#17225) Review of #18343: - 20261011_098 downgrade() dropped the four new columns, deleting every audit value written into them since the upgrade -- a deletion path in the audit store (#17038). It is now a documented no-op; the columns are additive and harmless to leave. The SLM migration has no down path and says it may not grow one that drops them. - Both index builds were plain CREATE INDEX, which blocks writes to audit_logs for the whole build. Alembic now uses postgresql_concurrently=True inside autocommit_block(); the SLM runner commits the columns and builds CONCURRENTLY IF NOT EXISTS on an autocommit connection. Both drop and rebuild an INVALID index a failed concurrent build left behind, so a re-run repairs rather than skips it. - Tests: downgrade has no statements; every create/drop index call is concurrent and inside autocommit_block; the SLM build runs in autocommit with CONCURRENTLY and rebuilds an invalid leftover.
…ound the orphan lock, lift the request cap off the background scan (#18013) _run_duplicate_analysis signalled 'lock held' and 'deadline expired' with the same None, so every caller reported 'timed out after 120s' - including a request that returned in 0.2s because a scan was already running, with advice (raise min_similarity) that cannot help. They are now ScanBusy (carrying the holder's age) and ScanTimedOut (carrying the deadline that actually elapsed), each with its own payload; report.py logs which. The deferred lock release (#13602) relied on the orphaned executor thread exiting. A thread that never exits left every later call declined for the life of the process. Past AUTOBOT_DUPLICATE_ORPHAN_LEASE_SECONDS after the scan was told to stop, the lock is reclaimed; the late callback releases the lock object it captured, so it can never free one a newer scan owns. The Celery task applied the 120s request deadline, so the queued path that exists to escape it timed out the same way and reported completed. It is now bounded by AUTOBOT_DUPLICATE_BACKGROUND_TIMEOUT_SECONDS and a timeout raises instead of returning a partial result (#18067). duplicates.py leaves its grandfathered ceiling (660 -> 470): result shaping and config-duplicate helpers moved unchanged to duplicate_results.py. Refs #18013, #18067
… failure, and a lookup that threw is not 'no such source' (#17982) Celery records a RETURNED value as SUCCESS whatever it says, so the three scan tasks that return a refusal dict (import_tree, duplicate, dependency, all through _scan_root_or_refusal) surfaced as completed / progress 100 / error None. celery_result_to_status now maps a payload whose own status is 'error' to failed and carries the reason to the status endpoint; this is the one shared mapper, so all three are covered and asserted, not only the duplicate one. The duplicate task also refuses a scan that examined zero files. /duplicates/cached answered HTTP 200 for a failed scan, which is how a consumer checking only the status code rendered it as 'no duplicates'. It is 409 now with a fixed message; the task's exception text is logged and served by the status endpoint, not echoed (CWE-209). resolve_source_root swallowed every lookup failure into logger.debug and returned None, and source_storage.get_source returned None when it could not build its Redis client. A fault and an unknown id were therefore the same observable, and the refusal asserted the id was unknown. Both now raise RegistryUnavailable, logged at error with the cause chained; the default-source lookup no longer falls through to AutoBot's own tree on a registry fault. A structural guard fails if any broad handler around a source lookup swallows instead of re-raising, anywhere in the package - the #2760 extraction left a copy behind because nothing guarded more than one file. Not established: why the source id did not resolve on the deployment that produced the original report. The swallowed exception was never logged above DEBUG, so the cause is unrecoverable; what is fixed is that the next occurrence names its cause. Refs #17982
…wn tree, and config-duplicates says when it read nothing (#18067) Why the 120s timeout and the zero results. Measured on the deployment's own backend log for the registered AutoBot source: 8,377 files -> 130,257 blocks -> an LSH index of ALL 130k blocks -> 3.96M candidate pairs -> 2.54M reported 'duplicates' (2.43M of them 'low'), about 9 minutes end to end. The request deadline (120s) always fired first, so the response was an empty 'partial'. Three causes, each independent: 1. LSH was built at a hardcoded 0.5 whatever min_similarity was, so the advice in every timeout message ('try a higher min_similarity') changed the answer but not the work. 2. Half of the blocks have <= 17 distinct tokens. Token-set Jaccard on sets that small reaches 0.5 by coincidence; they flooded the candidate set. 3. 'No artificial limit on returned pairs' (#609) meant 2.5M pairs were built, held, and returned - no panel, Redis result or JSON response can carry that. Fix, in a new duplicate_lsh.py (duplicate_detector.py was grandfathered at its size ceiling and shrinks 1326 -> 1120): the fuzzy pass takes blocks with >= 20 distinct tokens (exact-hash matching is unchanged and still covers every size); the index is built LSH_RECALL_MARGIN (0.1) below min_similarity because MinHash-LSH retrieves a pair AT the threshold only ~50% of the time; MinHash is one update_batch call per block; the query loop is cancellable. The returned list is capped at AUTOBOT_DUPLICATE_MAX_RETURNED (5000) while total_count and the high/medium/low statistics still describe every pair, and `truncated` says so. Measured on this repository after the change: 8,384 files, 175s, 56,557 pairs found (25,491 high), 5,000 returned, truncated=true - on the background path, which is bounded by AUTOBOT_DUPLICATE_BACKGROUND_TIMEOUT_SECONDS and not the request deadline. The synchronous GET still exceeds 120s on this tree and now says so honestly. Also fixed: detect_duplicates_async had no min_similarity parameter, so the one caller that passed it raised TypeError on every semantic request and silently fell back to the standard scan. config-duplicates: detect_config_duplicates scanned <root>/src and <root>/backend. AutoBot has neither, so it read zero files and answered 'No configuration duplicates found!' in 0.7s. It also skipped any file whose ABSOLUTE path contained 'test'. It now scans from the root when the legacy directories are absent, prunes SKIP_DIRS, judges test files on the relative path, counts files scanned and failed (parse failures were logged at DEBUG), and the endpoint answers 422 when it read nothing. Its report states how many files the zero is about. Known-answer fixtures: a planted duplicate pair that must be found and a near-miss that must not, for the block detector and for config-duplicates. Not done here, still open on #18067: the symbol-fork detector reaching analytics, the per-detector ran/zero/failed record for the cross-file rules, and the problem-type counts. Refs #18067, #18013, #17982
…an unreadable registry on purpose (#17982) get_source and get_default_source_id now raise RegistryUnavailable where they returned None. Audited every caller (backend, LLC, tasks; SLM and scripts have none): authorization answers 503 instead of a false 404, HTTP handlers resolve the default source through shared.default_source_id (503), an app-level handler turns any remaining fault into 503 without echoing the cause, the LLC project list degrades without the source summary, and project disposal aborts rather than orphaning the clone. Celery tasks let it fail the task. Refs #17982
…e registry reach its 503 handler (#17982)
… detector, the cross-file pass says what it ran, duplicate_abstraction is wired, type coverage is a number (#18067)
…ensus mid-window after the merged tree grew (#18293)
…nto issue-18067-duplicate-tooling
import-hermeticity 687 (population 717), one-claim-registry 3087 (3112), tracked-key-material 11044 (11444), each mid-window from the measurement.
Contributor
|
Important Review skippedToo many files! This PR contains 203 files, which is 103 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters (2)
📒 Files selected for processing (203)
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for name, create in _INDEX_SQL: | ||
| cursor.execute(_INVALID_INDEX_SQL, (name,)) | ||
| if cursor.fetchone(): | ||
| cursor.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name}") # nosec B608 -- name is a module constant |
Owner
Author
|
Superseded: rebuilt as train #6b without #18362 (fixed sleeps at duplicate_scan_outcomes_18013_test.py:122/137/156, a router mounted ungated on a bare app in registry_unavailable_503_through_decorator_17982_test.py:27, severity literals in fork_findings.py, and module-level sys.path.insert(0, …) in check_symbol_forks.py:55 and fork_findings_18067_test.py:29, the probable cause of the scripts.* collection errors in all 12 shards) and without #18343 (its _RETIRED list trips the stranded-recorder census). Both are back with their owner. |
Owner
Author
|
Correction to the comment above: #18362's |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thinking Path
Merge train #6, under the owner decisions of 2026-10-11 and 2026-10-12 (every approved PR lands through a train). It is stacked on train #5 (#18358): this CI measures
main+ train #5 + this train. Merge it only after #18358. It lands with a merge commit, and the drift-prone guards were pre-run locally on the combined tree.What Changed
Constituents (
#PR @ approved head):DELETE /chats/{id}awaits the one canonical delete. A real-manager test is included, plus an AST guard against un-awaited async ChatHistoryManager calls inapi/.audit_logsis the system of record. The migrations on both backends are additive with no-op downgrades, and the indexes are built CONCURRENTLY with INVALID-index repair.Train-only commits:
repo_tests/_reach_policy.pyandrepo_tests/reach_scope_claim_17844_test.py. The Python size baselines are set from measured line counts:analytics_service.py835, andaudit_logger.py(301, now under 600) is dropped.Closes #18013
Closes #18093
Closes #18124
Closes #18293
Closes #18316
Refs #15722
Refs #16375
Refs #17225
Refs #17982
Refs #18067
Refs #18315
Refs #18349
Verification
Model Used
Claude Opus 5.5 (merge orchestration). Constituent reviews used Sonnet and Opus by risk tier.
🤖 Generated with Claude Code