Feat/memory chat agent - #76
Merged
Merged
Conversation
- satisfy OpenAI strict structured output - fall back to plain JSON when the extraction schema is refused
…at tenant models, and tenant chat clients - Implement tests for memory settings in `test_memory_settings.py` to validate default values and environment overrides. - Create `test_retrieval_graph_seeds.py` to ensure graph observation and memory seeding functionality works as expected. - Add tests in `test_runtime_chat_tenant_models.py` to verify tenant-specific chat client configurations and model validations. - Introduce `test_tenant_chat_clients.py` to check the caching and disposal behavior of tenant chat clients, including handling of fingerprints and secrets.
…context request - Added `graph_seed_node_keys` to `RetrievalRequest` for additional observation nodes. - Introduced `MemoryContextRequest` dataclass for managing memory context parameters. - Updated `RetrievalFacade` to utilize new graph seed nodes in retrieval processes. - Integrated memory context service into `HarborRAG` for improved memory management. - Enhanced tests to validate new memory usage tracking and model telemetry. - Updated dependencies in `pyproject.toml` and `uv.lock` for langchain integration.
- Introduced `KnowledgeReferenceStore` and `ReferenceValue` for managing typed references. - Added `retrieval_cost.py` for handling retrieval cost accounting. - Created `retrieval_inputs.py` for shared input validation helpers. - Developed `retrieval_schemas.py` for canonical input schema builders for retrieval tools. - Implemented `source_list_tool.py` for permission-scoped source discovery. - Added `vector_search.py` for tenant-scoped vector retrieval with explicit controls. - Enhanced unit tests for agent tools and conversation coordination. - Updated document retrieval tests to ensure metadata accuracy and permission checks.
- Updated documentation to include chat release evaluation details. - Improved chat modes with RAG and agent modes for better handling of multi-document queries. - Modified reasoning effort handling in chat parameters to align with deployment capabilities. - Added tests for reasoning opt-out scenarios and validation of reasoning capabilities. - Introduced new scripts for controlled quality probing of chat models and API endpoints. - Enhanced agent execution to better manage tool calls and reasoning efforts. - Implemented checks for tool schema compliance and agent instructions clarity.
- Introduced new citation assessment logic to ensure only valid citations are included in agent responses. - Added tests for citation validation, ensuring that invalid markers are correctly identified and excluded. - Improved evidence extraction from tool results, ensuring only available and valid sources are cited. - Updated chat quality probes to reject responses lacking evidence or containing inconsistent citation counts. - Refactored citation-related functions for better clarity and maintainability.
… exports - Deleted compatibility exports in `references.py`, `__init__.py`, `base.py`, `catalog_factory.py`, `describe_graph.py`, `describe_graph_schema.py`, `evidence_output_schema.py`, `graph_catalog.py`, `graph_search.py`, `graph_search_support.py`, `output_schemas.py`, `reader_base.py`, `reader_catalog.py`, `reader_support.py`, `reader_tools.py`, `retrieval_inputs.py`, `source_list_tool.py`, and `vector_search.py`. - Updated imports in `base.py`, `server.py`, and various test files to use runtime tools directly. - Changed tool specifications in `McpServer` and related classes to use `ToolSpec` and `BaseTool` from the runtime. - Adjusted tests to reflect the new structure and imports from the runtime. - Enhanced error logging in `memory_tools.py` for better clarity on indexing failures. - Added new tests for conversation memory coordination in `test_in_memory_conversation_coordination.py`.
The shared ToolBudget landed with gaps that made the two transports disagree and, on the agent path, made three tools uncallable. - Declare jsonschema in harborrag-runtime. tools/budgets.py imports it at module level, so a wheel install without the mcp-server or an optional extra failed at import before any command ran. - Bind a tool call to a tenant only where the schema declares one. The engine injected tenant_id unconditionally, which failed additionalProperties on describe_graph and on both memory tools -- long-term recall was unreachable through the agent loop. Where the property is declared the write stays unconditional, so a model-supplied tenant is still overwritten. - Run check_call and check_results on the memory tools too. Their maxLength and additionalProperties bounds were advertised to the model and enforced by nothing; reject_owner_fields stays as the guard beneath. - Report the constraint, not the payload. A combinator failure rendered as repr(instance), so a 60 KB filters dict came back as a 60 KB error that the engine then truncated the explanation off the end of. - Cache compiled validators by canonical schema text. check_schema ran per call and dominated cost, ~1.7 ms per agent tool call and ~29 ms per MCP dispatch. - Make McpToolPolicy a relabelled ToolBudget and derive PolicyConfiguration's ceilings from the shared constants. The literal 20 matched MAX_TOOL_RESULTS by coincidence, so raising it would have made MCP reject calls its own advertised schema declared valid. - Map PermissionError to a model-visible error, and log a tool exception that escapes call_tool instead of discarding it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Updated the migration function reference in the readonly composition test to use the correct path. - Changed the method for building the topology repository in the same test. - Modified the dependency direction checks to remove unnecessary imports and clarify allowed package relationships. - Introduced a new script for smoke-testing package installations in isolated environments, ensuring that only the required packages and their dependencies are installed. - Cleaned up the `uv.lock` file by removing unused dependencies and ensuring correct package specifications.
Four places let something other than the caller's own identity decide what the
caller could read or write.
- Long-term memory checked a write with visible_to, which compares only the
fields the scope keys on. A TENANT-scoped write was therefore unconstrained
beyond the tenant: any member could publish a tenant-wide fact stamped with a
colleague's user_id. Erasure is by owner, so the colleague's erasure would
delete it and the author's would miss it. A write may still leave a field
unset, which is how a genuinely tenant-wide fact is recorded.
- Graph reads derived tenant visibility from the candidate batch's own
allowlists. A batch holding only tenant-scoped nodes produced empty lists and
denied them to a fully authorized reader, while one readable document
alongside them unlocked every one. It now asks the principal's own grants,
and only when the batch contains such a record.
- FastMCP defaults mask_error_details off, so any exception escaping a tool
reached the client as "Error calling tool 'x': {exc}" -- enough to leak a
connection URL or a server path. The HTTP route already masked; the MCP
transport now agrees. A ToolError we raise ourselves is still delivered whole.
- Resolving the MCP principal as a call argument put it before call_tool, so a
token probing another tenant left no audit record at all. The attempt and its
refusal are now both recorded against the token's subject.
Also return an unauthorized HTTP tool call as its own status instead of letting
the generic handler report an authorization decision as a 500.
Coverage was below its own 90% gate at 88.04%; these tests bring it to 90.08%.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four resource and durability defects, each on a path that only shows up under load or on someone else's filesystem. - Kill an isolated subprocess on every exit path. join() with a timeout returns whether or not the process ended, so a parse that stopped heartbeating was joined and then abandoned while still running. Only cancellation killed, and the document stage retries five times, so each hung parse piled another live interpreter onto the worker. - Bound concurrent document retirement. retire_removed opens a session per call and every removal went in flight at once, so a scan detecting a few hundred removals overran the control pool, timed out, and marked the task FAILED after every document had already been published. - Resolve the shared ingestion batch defaults without building a Temporal deployment. Reading two integers validated the whole config, so a stray HARBORRAG_TEMPORAL_TARGET without TLS, or a worker capacity exceeding the database pool, failed an inline ingestion that connects to neither. - Separate sqlite_url from prepare_sqlite_database. Building a URL created directories and reset the mode of whichever parent the operator had chosen, stripping group and other access from a shared directory or raising PermissionError on one this process does not own. A directory we create is still 0700 and the database file is still 0600. Behaviour change: a pre-existing parent directory now keeps its mode. Callers that relied on sqlite_url creating the file must call prepare_sqlite_database. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
User accounts are deliberately not enabled: Principal.__post_init__ pins user_id to DEFAULT_USER for every caller, and a test pins that decision. The surrounding documentation described the opposite, which is the part worth fixing without reopening the decision. Five docstrings claimed a per-person boundary that does not exist. The conversation and memory routes said a caller "cannot ask about or erase anyone else's" data; in HMAC mode any subject holding a token for a tenant can list, read, rename, delete and erase every conversation and USER-scoped memory in it. They now say so, and name the tenant as the boundary that does hold. HARBORRAG_AUTH_USER_ID_CLAIM is resolved by the verifier and then discarded. It is kept as the seam the user-accounts work will use, now marked as having no effect today rather than described as an isolation control. The capacity tiers have the same root. capacity_scope_for reads the pinned user_id, so the user tier is a per-tenant limit: two subjects key one bucket and one of them holding several long-lived streams exhausts it for the other. The "identical keys collapse" tests describe a scope no API caller reaches, because user_id never equals the subject. Added the missing test that drives capacity_scope_for from a real Principal and pins what actually happens. No behaviour changes here. Per-user ownership and a real per-user capacity tier both need the user-accounts decision, not a configuration change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Binding a tenant only where the schema declares the property turns an omitted
tenant_id into a silent grant rather than a loud failure: ToolSpec.input_schema
defaults to a bare {"type": "object"}, so a tool added without a schema would
receive the model's raw arguments unscoped and nothing would complain. The
catalog test names the three tools that are legitimately tenant-free, with the
reason each one is, and fails the day a fourth appears. A second test holds the
other half of their safety: all three forbid additional properties.
Also removed a sentence added in the previous commit claiming callers can set
limit_scope to principal. limit_scope is only ever written into
HarborRateLimitError.details to report which tier was exceeded; it is not an
input. Replacing prose that promised what the code does not do was the point of
that commit, so it should not have introduced more.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Security definitions first. Configuration validation, model-error sanitisation
and text redaction each kept their own list of credential-shaped field names,
and they had already drifted: only redaction knew private_key, none knew
passwd. They now share SENSITIVE_FIELD_TOKENS.
Consolidating surfaced three real gaps, each fixed:
- A compound name was compared against the whole field name, so api_key was
caught and my_api_key was not. A prefix was all it took to store a raw key.
Compound entries now match at token boundaries.
- redact_secrets only handled "authorization: Bearer x", so a Basic credential
or a raw key under that header went to the log intact.
- "token" also means an LLM accounting unit, so {"token_budget": 8000} was
unstorable while max_tokens passed on the plural. Counting qualifiers now
exempt it, and "tokens=5" is no longer rewritten to "token=<redacted>".
Behaviour change: private_key, passwd and any compound spelling of an existing
name are now refused in stored configuration. A deployment holding one of those
raw must move it behind a secret:// reference.
Contracts that only held by accident:
- URLPolicy.validate let urlparse's bare ValueError escape for a malformed URL,
so a caller mapping URLPolicyError to a 400 got an unhandled 500 and one
using it as a deny path treated the URL as acceptable.
- finish_reason was str | FinishReason, which a smart union always resolved to
str, so comparing against the enum was False for every response. It is the
enum now, with a lenient parse so an unfamiliar provider value becomes
UNKNOWN rather than failing a call that succeeded.
- ConversationIdentity and AgentRunIdentity accepted blank isolation keys and
pooled every such caller into one "" bucket. principal_id stays exempt: it is
audit provenance, excluded from comparison, and callers legitimately leave it
blank when building an identity purely as a lookup key.
- _deep_freeze recognised FrozenMetadata by comparing __name__ and __module__,
so moving or subclassing it silently resumed re-wrapping.
- The agent evidence marker was guarded by assert, which -O erases.
Not fixed, needs a decision: SettingsRepositoryPort is not tenant-scoped even
though its provider is tenant-scoped and WorkspaceSettings carries a tenant_id,
so every tenant shares one document. Closing it needs a unique constraint on
workspace_settings.tenant_id and a migration to split the existing row, so the
port now documents the gap rather than half-closing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Recall deduped on content_hash, which is salted with the scope. One sentence stored at both SESSION and USER hashed differently, so both survived: the budget was spent twice and the reader saw one fact presented as two. Rows with no hash already deduped on normalized content, so the rule also depended on whether a hash happened to exist. It is the normalized content either way now. Add-only extraction built its hash set from at most fifty rows per scope with no ordering guarantee, so past fifty memories in a scope the "a retry is a no-op" guarantee lapsed and restatements accumulated. It now asks about the one fact being written; the semantic check stays as the backstop for differently worded restatements. The in-memory working store evicted an expired entry only when that exact owner was read again, so a run that ended without one more read kept its scratch state for the life of the process. Writes now sweep. Reading a conversation raised on a single unparseable citations or tool-calls column, making the whole history unreadable. A message with unreadable metadata is worth more than no conversation, so the metadata is dropped and the text kept. snapshot(query=...) returned () when no long-term tier was configured while search() raised, so callers could not tell "nothing stored" from "not configured". Both raise now. Three copies of the rule mapping a memory owner onto its conversation identity, comment and all, are now one shared function. Removed the unused record, remember and forget aliases. Behaviour changes: recall returns fewer rows where the same fact was filed under two scopes, and snapshot now raises instead of returning empty. Refuted while checking, so deliberately unchanged: the rolling summary does trigger for a window of short messages. Reproduced with thirty ten-token messages against a twelve-message window -- the catch-up path writes a summary covering the older turns and LAST_COVERED_KEY is read back on the next build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A resume restored only logical_model and took graph_search, max_steps, max_total_tokens and timeout_seconds from whoever called resume. Resuming with graph_search=True handed the model graph tools that the earlier steps of the same transcript never had, and a different budget silently re-budgeted a run already in progress. Those options are now checkpointed and restored. The checkpoint state is a JSON column, so no migration: a field the checkpoint does not carry predates this and falls back to the caller's value, which is what the run effectively had before. A terminal checkpoint that failed to save was suppressed without a trace, so the run stayed RUNNING under a live lease until it expired and a resume meanwhile failed with a misleading conflict. Still best effort, now logged. Cancellation checkpointing suppressed BaseException, which also swallowed a KeyboardInterrupt or SystemExit raised inside the shielded persist. Narrowed to Exception. The grounding instruction had lost its subject in an edit: "...alter a marker. distinguish any inference." The clause telling the model to separate evidence from inference is restored. The repeat-detection key was built by the same three-line expression at each of the three call sites, so the key a call was admitted under, executed under and rejected under could drift while every site still looked right alone. Authoritative retrieval doubled its window on each pass, re-reading the whole window from the start because a larger top_k re-ranks globally rather than paging. It now widens to what the observed acceptance rate implies, reaching the same window in about one extra read instead of three. Removed identity_rewrite and keep_top, which only their own tests called. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FalkorDB's generic graph repository opens one client on one graph and has never honoured tenant_isolation; only the topology and knowledge repositories do. Accepting the setting and ignoring it is the dangerous half: an operator who switched it on believed tenants sat in separate graphs while every write still went to the shared one, separated by predicate alone. Construction now refuses. Hybrid search caps its candidate set because fusion holds both lanes in memory, then sliced the requested page out of it. A page past the cap returned the empty slice, reporting "no more results" for a page never looked at. It now raises a capability error naming the ceiling. delete_records was the one Qdrant operation that skipped the spec lookup, so deleting from an unknown index produced a raw provider 404 instead of HarborStorageNotFoundError. A provider signalled a missing optional extra by raising ImportError with an exact wording the registry matched by regular expression, so a provider wording it differently, or an ImportError from a transitive import, fell through as a bare traceback. There is a MissingOptionalDependencyError now; the message is unchanged and the regex remains as the compatibility path. cryptography and alembic were imported unguarded and so never reached that path at all. Behaviour change: a deployment with falkordb tenant_isolation set on the generic graph repository now fails at construction instead of starting up unisolated. Investigated and deliberately not changed: driver exceptions crossing the storage port. Twenty-two tests across S3, redis and Qdrant assert that an unmapped provider error is re-raised as itself, so this is a tested contract rather than an oversight. Translating it is a design decision, not a fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vector_search's score_threshold filtered on score, which the settings comment and the output schema both say must never be thresholded: on the hybrid lane it is rank-fusion arithmetic whose top hit sits near 1.0 however poor the match. A request for high-quality results therefore returned that hit regardless of quality and dropped good matches further down. It filters on relevance now, falling back to score only for lanes that report no relevance at all, and the parameter says so to the model. Three tool modules imported their contract types from the SDK facade rather than from reader_contracts where they are defined, so importing a stateless tool pulled in HarborRAG, chat, memory and execution -- and made a cycle inevitable the moment the SDK referenced tools at runtime. The reference store swept expired handles on every issue by walking every entry, which at the hundred-thousand ceiling meant a full scan under the lock per tool call. Entries share one TTL and re-issuing does not extend it, so insertion order is expiry order and the first live entry ends the sweep. The settings module claimed pydantic-settings was an optional extra imported lazily by CompositionRoot.production(). It is a required dependency imported eagerly, which matters to anyone deciding where a new import may go. Behaviour change: a vector_search call with a non-zero score_threshold now returns results chosen by measured relevance, so a hybrid-lane query returns a different set than before. Left in place after investigating: the hasattr guard in the Temporal worker. It is duck-typing for settings objects without graph-build configuration, not a dead branch, and removing it pulls an optional projection queue into a test that deliberately does not exercise it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The trail recorded a tool, a principal and an argument digest, but not the tenant -- the first question anyone asks of it. Both events now carry the canonical stripped tenant, the same value that drove policy, validation and the grant check. The arguments stay a digest so the log never becomes a copy of the payload. Its default path was relative, so an MCP client launching the stdio server from a directory of its choosing put the trail somewhere different each time, or in a directory the process could not make owner-only, which failed every tool call. A relative path is now anchored at the home directory, where the ownership rules the writer enforces can actually hold. An absolute path is untouched. Each durable event opens, writes and fsyncs under a lock, twice per call, and dispatch runs inline in the transport's loop. The writes move to a thread; fsync stays, because an audit trail that is fast and lossy is worth less than one that is slow and complete. The audit counted status == "error" as a failure while the MCP handler raised only on ok is False, so the two could disagree about one result. One predicate decides for both now. Reading the cached configuration deep-copied the whole document, and a tools/list resolves every tool twice and asks for the policy once per tool -- roughly forty copies per listing. Internal readers use the cached object and still copy what they hand back. Documented rather than changed: FastMCP builds its tool table at startup, so tools/list cannot vary per tenant. A tool a tenant has disabled is advertised and then refused at call time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PATCH /api/v1/sources/{id} took status as an unvalidated string and wrote it
straight into a field whose domain type is a Literal on a plain dataclass,
which is a static annotation and no runtime check. {"status": "banana"}
persisted, and any scheduler keyed on active/paused/error then mishandled the
row. The input now carries the domain type, so an unknown value is a 422.
The workspace settings document is schemaless, so a webhook URL or a key can
end up in it, and it was returned verbatim to the lowest role that can reach
the route. It is redacted now, which is what the TODO there asked for.
The ingestion router held the server deadline at request scope, which the
capacity module's own docstring says must not wrap a streaming body: it stayed
armed across the SSE response and hard-cancelled the stream mid-flight with no
terminal error frame. Function scope, like the chat and agent routers. The
capacity lease is unaffected and still runs to the last frame.
The unified completions endpoint opened a session with kind "chat" whatever the
mode, so an agent run was filed as a chat and never appeared under
GET /v1/conversations?kind=agent.
manifest_id and generation_id were accepted by the legacy ingestion schema and
never forwarded, so a client that set one believed it had pinned a manifest
while nothing had. They are marked deprecated in the schema, which is what the
endpoint can honestly say about them.
Behaviour changes: an unknown source status is now rejected rather than stored,
and settings values under credential-shaped keys come back redacted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The route it is served from carries a reader-role dependency, and the document is schemaless, so a webhook URL or a key stored in it was readable by the lowest role that can reach the endpoint. The redaction the TODO there asked for now runs. Split from the previous commit, where the edit was lost to a failed assertion earlier in the same script. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running every package in a single pytest process failed twelve tests that pass per-package. None was a product defect; all three causes were test isolation. An app fixture set HARBORRAG_CONTROL_DB_URL to a tmp-path database through monkeypatch, while a sibling autouse fixture snapshotted os.environ to undo writes that production code makes directly. The snapshot was taken after the set, and its teardown can run after monkeypatch has already undone it, so it put the tmp path back permanently and leaked one test's database into every suite that followed. The snapshot fixture now runs first. The two runtime checkpoint tests that read that variable also unset it themselves, since what they assert is that no DSN is configured. The process-logging tests restored the shared "harborrag" logger afterwards but not before. configure_logging is deliberately idempotent, so a handler left by anything that ran earlier made the call a no-op and the test's own stream received nothing. The fixture now resets before the test too. The Temporal sandbox tests failed once fastmcp had been imported into the same process: the sandbox re-imports a workflow's dependency chain, and beartype does not survive it -- its claw submodule is left partially initialized. A worker never imports fastmcp, so this is an artefact of sharing an interpreter. A shared fixture passes beartype through, which is what passthrough is for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…que error classification
- Introduced new session management endpoints for listing, creating, renaming, and deleting sessions. - Added support for session history retrieval and pagination. - Enhanced API schemas to include new session-related fields and constraints. - Implemented query scope validation to ensure requests are properly classified. - Created TypeScript client methods for session operations, including error handling and request cancellation. - Added comprehensive tests for session management and query scope functionality. - Updated OpenAPI documentation to reflect new endpoints and parameters.
… agent completions
- Updated the HarborRAG class to lazily initialize memory and chat services. - Removed unnecessary imports and cleaned up the code structure. - Deleted unused tool modules and refactored related tests to reflect changes. - Adjusted dependencies in pyproject.toml and uv.lock for better package management. - Enhanced test coverage for reader applications and retrieval factory. - Ensured proper closure of services in tests to prevent resource leaks.
- Added HarborAuthorizationUnavailableError to handle unavailable permissions in retrieval. - Updated RetrievalPermissions to raise an error instead of returning empty results when authorization fails. - Improved handling of tenant shared corpus mode in retrieval permissions. - Refactored service closure logic to handle exceptions more gracefully. - Enhanced summary factory to support tenant-specific processing policies. - Introduced new environment handling for MCP server configuration. - Added support for hashed, tenant-bound API keys in MCP server. - Implemented tests for shared reader access and API key verification. - Updated deployment scripts to streamline environment variable management.
…y tool catalog integration
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 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 |
HuntyAmour
force-pushed
the
feat/memory-chat-agent
branch
from
September 18, 2026 14:41
9c851c3 to
7396fec
Compare
…nt completion response schema in tests
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
1 participant
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.
Pull Request
Summary
Describe the change in 1–2 sentences.
Type of change
Docs Impact (required for any code or docs changes)
docs/?python website/build.py+python website/check_links.py)Testing
Describe how you tested this change. Include commands and results.
Checklist
pytest -v)