David/HERMES-GIELIE
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

perf(webui/session-load-latency) tier3d: Greptile P1 followups — inconclusive scene check + full-content config verification

Browse source 
Two Greptile P1 comments from review of b5c0447a (tier3c):

1. Inconclusive Prefix Stays Fresh (discussion_r3548477859):
   When _persisted_session_meta_prefix() returns None and cached has scene
   records, the fast-path skipped scene comparison entirely and returned
   False (cache fresh) for inactive equal-count sessions. Added
   _scene_check_inconclusive flag that forces fall-through to the full
   Session.load_metadata_only() comparison when the cheap prefix read
   fails, preventing stale anchor_activity_scenes.

2. Prefix Check Misses Edits (discussion_r3548477915):
   The 128-char content-prefix backstop in _read_profile_config_cached
   only detected edits within the first 128 bytes of config.yaml. Replaced
   with a full-content comparison that reads the entire file (~1-10KB)
   and compares it to a copy stored in the cache entry. This detects
   edits anywhere in the file at a fraction of the cost of yaml.safe_load().
   Updated cache type hint from tuple[float, str|None, dict] to
   tuple[float, str, dict].

Tests: 108 passed (41 reconciliation + 39 anchor-scene + 28 profile-config)
This commit is contained in:
Konstantin M 2026-07-09 03:08:11 +00:00
parent b5c0447a4a
commit db99f72505
2 changed files with 46 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
api/models.py
View file

@ -3127,6 +3127,10 @@ def _cached_session_lags_disk(cached) -> bool:
cached_scenes = getattr(cached, 'anchor_activity_scenes', None) or {}
if not isinstance(cached_scenes, dict):
cached_scenes = {}
# Track whether the cheap scene check was inconclusive — when it is,
# we must fall through to the full metadata comparison instead of
# returning False for inactive sessions with matching counts.
_scene_check_inconclusive = False
if cached_scenes:
disk_meta_quick = _persisted_session_meta_prefix(sid)
if disk_meta_quick is not None:
@ -3160,6 +3164,13 @@ def _cached_session_lags_disk(cached) -> bool:
if _max_updated(disk_scenes) > _anchor_scene_records_updated_at(cached):
return True
# disk has no scenes, cache does -> cache is ahead; keep it.
else:
# Can't cheaply verify scene freshness from disk. The prefix
# reader may have failed while _persisted_message_count
# succeeded via index fallback. Fall through to the full
# metadata load so we don't serve stale scenes on the next
# equal-count inactive session path. Greptile P1.
_scene_check_inconclusive = True
else:
# Cached session has no scene records. Check if disk has gained
# the first scene record — without this the fast-path would miss
@ -3173,6 +3184,11 @@ def _cached_session_lags_disk(cached) -> bool:
# Active session: messages may be in flight; fall through to the
# full check to be safe.
pass
elif _scene_check_inconclusive:
# Could not cheaply verify scene freshness from disk; fall through
# to the full metadata comparison rather than returning False
# (which would serve a potentially stale cache). Greptile P1.
pass
else:
# Inactive session, count matches, scene records match — cache is
# at parity with disk.

56
api/routes.py
View file

@ -6314,32 +6314,34 @@ def _read_profile_model_config(
# new inode on Linux). mtime+size auto-invalidates on in-place edits; a 60s TTL
# is the backstop in case of coarse mtime resolution (some network filesystems
# round mtime to whole seconds — the size guard catches a write of equal-length
# bytes within the same second). On cache hit, a quick content-prefix comparison
# (first 128 chars) catches in-place rewrites that inode+mtime+size miss on
# coarse filesystems (Greptile P1, PR#5803 discussion_r3548069113).
# Reads are guarded by a single Lock to keep the hot path simple;
# the underlying yaml.safe_load is the slow step, not the lock, so contention
# is bounded.
_PROFILE_CONFIG_CACHE: "dict[tuple, tuple[float, str | None, dict]]" = {}
# bytes within the same second). On cache hit, a full-content comparison catches
# any in-place rewrite that inode+mtime+size missed (Greptile P1, PR#5803
# discussion_r3548477915). Reading and comparing the full file content (~1-10KB)
# is much cheaper than yaml.safe_load(). Reads are guarded by a single Lock to
# keep the hot path simple; the underlying yaml.safe_load is the slow step, not
# the lock, so contention is bounded.
_PROFILE_CONFIG_CACHE: "dict[tuple, tuple[float, str, dict]]" = {}
_PROFILE_CONFIG_CACHE_TTL_SECONDS = 60.0
_PROFILE_CONFIG_CACHE_LOCK = threading.Lock()
def _read_profile_config_cached(profile_name: str, cfg_path: str) -> dict | None:
"""Return parsed profile config, caching by (inode, mtime, size) with
TTL backstop and content-prefix verification.
TTL backstop and full-content verification.
The content-prefix comparison (first 128 chars of the file) catches
in-place rewrites where inode+mtime+size are identical on coarse
filesystems — most editors use atomic-rename (new inode), but tools
like sed -i or file.write() in-place can keep the same inode.
The full-content comparison reads the current file and compares it to a
copy stored in the cache entry. This catches any in-place rewrite where
inode+mtime+size are identical, regardless of where in the file the change
occurs — unlike a fixed-length prefix comparison, edits to fields after the
first N characters are always detected. Reading and comparing the full file
content (~1-10KB for a typical config.yaml) is much cheaper than
yaml.safe_load().
NOTE: This is a best-effort backstop, not a full integrity check.
Edits that change bytes only after the first 128 characters while
keeping inode+mtime+size identical are not detected until the TTL
expires. This trade-off is acceptable because the TTL is only 60s
and the common case (atomic-rename editors) is fully handled by
the inode key component.
NOTE: The cache key uses inode+mtime+size to handle the common cases
(atomic-rename editors -> new inode; in-place editors -> mtime/size
change). The full-content comparison is a backstop for the rare case where
all three collide (e.g., sed -i on a filesystem with coarse mtime
resolution, writing the same byte count).
"""
try:
st = os.stat(cfg_path)
@ -6353,17 +6355,20 @@ def _read_profile_config_cached(profile_name: str, cfg_path: str) -> dict | None
with _PROFILE_CONFIG_CACHE_LOCK:
cached = _PROFILE_CONFIG_CACHE.get(key)
if cached is not None:
cached_at, cached_prefix, cached_dict = cached
cached_at, cached_content, cached_dict = cached
if (now - cached_at) <= _PROFILE_CONFIG_CACHE_TTL_SECONDS:
# Cheap content verification: read first 128 chars to catch
# in-place rewrites that inode+mtime+size don't detect.
_current_prefix = None
# Full content comparison catches any in-place rewrite where
# inode+mtime+size are identical but the file content changed.
# Reading and comparing the full file (~1-10KB) is cheaper than
# yaml.safe_load(). Unlike a fixed-length prefix, this detects
# edits anywhere in the file. Greptile P1 (PR#5803).
_current_content = None
try:
with open(cfg_path, "r", encoding="utf-8") as _f:
_current_prefix = _f.read(128)
_current_content = _f.read()
except Exception:
pass
if _current_prefix == cached_prefix:
if _current_content == cached_content:
return cached_dict
# Content changed while key collided — fall through to re-parse
import yaml
@ -6375,9 +6380,8 @@ def _read_profile_config_cached(profile_name: str, cfg_path: str) -> dict | None
return None
if not isinstance(parsed, dict):
return None
content_prefix = content[:128] # first 128 chars for change detection
with _PROFILE_CONFIG_CACHE_LOCK:
_PROFILE_CONFIG_CACHE[key] = (now, content_prefix, parsed)
_PROFILE_CONFIG_CACHE[key] = (now, content, parsed)
# Cap the cache at 32 entries; profiles are bounded in practice
# and unbounded growth would be a leak.
if len(_PROFILE_CONFIG_CACHE) > 32: