* fix(agno): surface unevaluable policy rules (#778)
* fix(agno): fixed qodo reviews
check_policy previously let unevaluable policy rules silently return
compliant=True with no signal (issue #778): a rule referencing a field
missing from decision_data, or a rule string not matching the expected
<field> <op> <value> format, both fell through _eval_rule's `return
True` and were treated as passed.
Both now raise ValueError, which routes through check_policy's existing
exception handler and surfaces as a `warnings` entry instead. compliant/
violations semantics are unchanged for every case that previously worked
correctly; an unevaluable rule is not counted as a violation since it's
genuinely unknown whether it would have passed.
Follow-up fixes from code review:
- policy_rules decoded via json.loads without checking it was a list;
a JSON-encoded bare string decoded to a Python str, so iterating it
evaluated one "rule" per character, amplifying a single input-shape
mistake into a wall of per-character warnings. A decoded string is
now treated as a single rule; any other non-list shape or non-string
list element produces exactly one warning instead.
- _eval_rule used `data.get(field) is None` to detect a missing field,
which can't distinguish an absent key from a key present with JSON
null - both produced the same "undefined field" warning. Field
presence is now checked with `field not in data` first, and a
present-but-null value gets its own distinct message.
Added regression tests for all of the above in
tests/integrations/agno/test_decision_kit.py (38 tests in the file,
128 passing across tests/integrations/agno/).
* fix(agno): reject non-object decision_data in check_policy
check_policy only validated that decision_data was well-formed JSON,
not that it decoded to an object. When it decoded to a list, `field
not in data` in _eval_rule silently became list-membership testing
of values instead of a dict key check - e.g. "confidence" not in
["confidence", 0.95] evaluates to False - so a matching rule fell
through to data["confidence"], raising a raw internal TypeError
("list indices must be integers or slices, not str") instead of any
meaningful diagnostic. Numbers, strings, and bools produced similarly
opaque TypeErrors deep inside _eval_rule.
check_policy now checks isinstance(data, dict) right after decoding
and rejects any other shape with a single clear violations entry,
the same way it already rejects malformed JSON.
Added 5 regression tests in tests/integrations/agno/test_decision_kit.py
covering list/number/string/bool/null decision_data shapes (43 tests
in the file, 133 passing across tests/integrations/agno/).
Addresses Copilot PR review comment on the #778 fix branch.
* docs(changelog): reference PR #822 in the check_policy changelog entry
---------
Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
* fix(provenance): log tracking failures and return None on storage error (closes#783)
* docs(provenance): document Optional return types and failure behavior (#783)
* fixed qodo reviews
- split.ProvenanceTracker: Check _unified_manager.track_chunk() return value and fall back to legacy storage when None is returned (storage failure)
- split.ProvenanceTracker: Initialize legacy stores (_provenance_store and _chunk_registry) unconditionally in __init__ so legacy fallback storage works safely even when initialized with use_unified=True
- split.ProvenanceTracker: Update get_provenance() to check legacy storage when no record is found in unified storage, ensuring fallback-tracked chunks remain retrievable
- SourceTracker: Change track_sources_batch() condition from if entry is not None: to if ok: to respect the boolean return contract (-> bool) of track_entity_source(), track_property_source(), and track_relationship_source()
- Tests: Add regression test test_unified_tracking_returns_none_triggers_fallback and update test_track_sources_batch_failure_not_counted to test boolean failure return_value=False
---------
Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
- Removed unused `validate_skos_hierarchy` import from
test_ontology_subissue3.py (flake8 F401); the test uses a `wraps=` spy
on the real add_nodes_and_edges instead of calling the helper directly.
- refresh_ontology tests now percent-encode the ontology URI with
urllib.parse.quote before interpolating it into the {ontology_uri:path}
request path, matching the already-encoded unknown-uri refresh test in
the same file instead of embedding a raw http://... URI with slashes.
- Reworded the cyclic-SKOS refresh test's comment and section header:
GraphSession.add_nodes_and_edges() documents pre-write validation and
lock-based mutual exclusion, not transactional rollback, so "atomic"
was replaced with "single combined add_nodes_and_edges() call" to avoid
implying rollback guarantees that don't exist.
Verified: tests/explorer/test_ontology_subissue3.py (34 passed) and
tests/explorer/ (204 passed), no regressions.
- validate_skos_hierarchy() re-walked every existing hierarchy edge in
the graph on each write, so one pre-existing cycle anywhere would
block all unrelated future SKOS writes. It now only traverses
concepts touched by the edges actually being written, while still
checking those against existing edges for cross-boundary cycles.
- In /api/ontology/load, `except HTTPException: raise` sat after a
broader `except Exception` clause that already matched HTTPException,
so a 422 raised after a successful OntologyIngestor parse was
silently swallowed and retried via the fallback RDF parser instead of
reaching the caller. Reordered the except clauses.
Co-authored-by: mikemikimike <13286568797@163.com>
- Added exc_info=True to both store failed and record_decision failed warning logs in _AgentScopedStore.upsert_memory() to preserve full traceback context for debugging
- Updated CHANGELOG.md entry to document traceback preservation
- split.ProvenanceTracker: Check _unified_manager.track_chunk() return value and fall back to legacy storage when None is returned (storage failure)
- split.ProvenanceTracker: Initialize legacy stores (_provenance_store and _chunk_registry) unconditionally in __init__ so legacy fallback storage works safely even when initialized with use_unified=True
- split.ProvenanceTracker: Update get_provenance() to check legacy storage when no record is found in unified storage, ensuring fallback-tracked chunks remain retrievable
- SourceTracker: Change track_sources_batch() condition from if entry is not None: to if ok: to respect the boolean return contract (-> bool) of track_entity_source(), track_property_source(), and track_relationship_source()
- Tests: Add regression test test_unified_tracking_returns_none_triggers_fallback and update test_track_sources_batch_failure_not_counted to test boolean failure return_value=False
Signature introspection and the resulting call were sharing one
try/except, so a genuine bug inside a backend's get_causal_chain
(raising an unrelated TypeError) was misread as a signature mismatch,
causing an identical retry call before the real error surfaced.
Split introspection from the call so a successfully-introspected call
happens exactly once; the trial-and-error cascade now only runs when
inspect.signature itself fails. Also adds the CHANGELOG entry for
#781/#817, which was missing.
- Add safe input validation and bounds clamping on max_depth (1..100) to prevent DoS/memory exhaustion
- Use inspect.signature for accurate keyword dispatch with precise TypeError fallback
- Prevent masking of genuine internal TypeError exceptions inside graph backends
- Add security and input hardening regression tests
- Support legacy (depth kwarg) and positional-only get_causal_chain backend signatures in fallback path
- Add regression tests for signature compatibility
- Return explicit error dictionary when graph lacks get_causal_chain instead of silent empty list
- Forward direction and max_depth in fallback graph.get_causal_chain call
- Add regression tests for error signaling and parameter forwarding
* feat(triplet_store): add Altair Anzo triplet store backend
Adds AnzoStore as a fourth peer to BlazegraphStore/RDF4JStore/JenaStore,
speaking plain SPARQL 1.1 over HTTP (no new dependency needed). The one
structural difference from the existing backends is that Anzo addresses
data by a dataset/graphmart URI rather than a short namespace/repository
name, so the endpoint path percent-encodes it. Reuses the shared
sparql_escaping.py helpers and wires "anzo" into TripletStore's backend
dispatch and config env vars.
Closes#813
* fix(triplet_store): correct AnzoStore SPARQL syntax and validate IRIs
Addresses review findings from Qodo and Codex on PR #814:
- get_triplets(): constraints are now expressed via FILTER(...) instead of
bare equality expressions appended inside the WHERE group graph pattern
(e.g. "?s ?p ?o ?s = <...>"), which is not valid SPARQL and was rejected
by standards-compliant endpoints.
- bulk_load(): named-graph inserts now nest the GRAPH block inside the
INSERT DATA braces (INSERT DATA { GRAPH <g> { ... } }) per the SPARQL 1.1
Update grammar, instead of "INSERT DATA GRAPH <g> { ... }".
- bulk_load()/_build_insert_data()/delete_triplet()/get_triplets() now
validate subject/predicate/graph URIs via sparql_escaping.validate_uri
before interpolating them into SPARQL Update/Query strings, closing an
injection path where a value containing ">" or "}" could break out of
the intended <...> token.
- Corrected the store_type docstring/usage example: Anzo's linked-data-set
store type is "lds", not "dataset".
Extended tests/triplet_store/test_anzo_store.py with coverage for the
corrected query shapes and the new validation/injection-rejection paths
(38 tests total, up from 32). Full tests/triplet_store/ suite: 299/299
passing.
* test(triplet_store): expand AnzoStore regression coverage
---------
Co-authored-by: Sameer6305 <sskadam6305@gmail.com>
* refactor(provenance): centralize duplicated store-and-swallow logic into _save_entry() (closes#784)
- Added ProvenanceManager._save_entry(entry) as the single shared
checksum-compute + storage.store() + graceful-failure-swallow
pipeline, previously duplicated identically across track_entity,
track_relationship, track_chunk, and track_property_source.
- track_entities_batch/track_chunks_batch already delegate to
track_entity/track_chunk in a loop, so they inherit the fix for
free — left untouched, confirmed no direct duplication there.
- Byte-for-byte preserves today's swallow-and-continue behavior and
comment text; this is an architecture-only refactor. The silent-
failure behavior itself is unchanged and out of scope here — a fix
to it now only needs to happen in one place instead of four.
- Added 4 new regression tests (previously 0 of the 4 single-item
methods had failure-path coverage) proving storage.store() raising
is still caught and each method still returns its ProvenanceEntry.
Tests: tests/provenance/ 228 passed (+4 new), tests/explorer/test_provenance_manager_wiring.py 8 passed. 236/236, 0 failed.
* fix(provenance): drop out-of-transaction store attempt in track_entity fallback
The _save_entry refactor changed track_entity's pre-build exception
fallback (entry is None branch) to call _save_entry(), which makes a
real self.storage.store(entry) call. The original code only computed
a checksum here and never attempted storage again, since this branch
fires when something already failed before the entry was built inside
the atomic transaction. Storing outside that transaction bypasses the
BEGIN IMMEDIATE serialization #807 added, risking the same race it
fixed. Restored checksum-only behavior and added a regression test
asserting storage.store is not called on this path.
Also removed an untested hasattr(_store_with_conn) defensive branch
added during the refactor that wasn't in the original code, and added
a changelog entry.
---------
Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
Two review findings on #807/#812:
- retrieve() and trace_lineage() were routed through transaction()'s
BEGIN IMMEDIATE, so plain reads took SQLite's writer lock and
serialized behind every other read/write, defeating the WAL
concurrency this PR was meant to add. They now use a dedicated
_read_connection() (configured, no explicit BEGIN).
- track_entity()/track_chunk() swallowed all internal storage
exceptions unconditionally, so a single item's failure inside
track_entities_batch()/track_chunks_batch()'s shared transaction
never reached the batch loop's per-item except, inflating
tracked_count for entries that were never persisted. Both now
re-raise when called with a shared _conn (batch context) while
still degrading gracefully on standalone calls.
Added regression tests for both, corrected the CHANGELOG entry and
docs that described the prior (overly broad) behavior.
Mohd Kaif