Skip to content

fix: address ByteRover review feedback#5

Merged
xraysight merged 1 commit into
mainfrom
fix/byterover-review-feedback
May 21, 2026
Merged

fix: address ByteRover review feedback#5
xraysight merged 1 commit into
mainfrom
fix/byterover-review-feedback

Conversation

@xraysight

Copy link
Copy Markdown
Owner

Summary
This is a follow-up to #4 addressing the automated Codex review feedback for the ByteRover dashboard integration.

Changes

  • handle ByteRover JSON-lines query output by selecting the final completed event payload
  • keep support for the existing single-JSON output shape
  • fix .brv/context-tree file containment checks using real path ancestry instead of string-prefix matching
  • remove redundant --project-root from brv status calls and rely on the configured project-root cwd
  • add regression tests for all three cases

Why
Codex flagged three issues in #4:

  1. brv query --format json may emit newline-delimited JSON events rather than one JSON document.
  2. The previous path containment check used startswith, which could be bypassed by sibling prefixes such as .brv/context-tree2.
  3. brv status --project-root ... may not be supported by older ByteRover CLI versions; since the subprocess already runs with cwd=project_root, the flag is unnecessary.

@xraysight xraysight merged commit a62dbd3 into main May 21, 2026
terencez127 added a commit to terencez127/hermes-memory-ui that referenced this pull request Jun 29, 2026
Fixes from code review of the initial OSS patch:

#1 & xraysight#2 - Silent swallow + duplicate import bug:
  Hoist MemoryConfig import above both try blocks, catch ImportError
  explicitly with a user-facing error. Replace 'pass' fallthrough
  (which silently called Memory(config=raw_dict) and always crashed)
  with an explicit error message returned to the dashboard.

xraysight#3 - mode:oss with flat config layout ignored:
  When mem0.json uses {"mode":"oss", "llm":{...}} (flat, no "oss"
  sub-block), oss_config was None and Memory() was called with no
  config at all, ignoring user settings. Now: flat layout is detected
  and the whole file_cfg (minus "mode" key) is used as oss_config.

xraysight#4 - agent_id asymmetry between OSS and cloud scope:
  OSS was always injecting agent_id="hermes" (the default) into
  filters, causing OSS to return a narrower result set than cloud for
  the same user. Now: agent_id filter only applied when explicitly set
  to a non-default value, matching cloud path behaviour.

xraysight#5 - rerank not forwarded to OSS search:
  Memory.search() supports rerank=bool. Now forwarded consistently
  with the cloud path.

xraysight#6 - double file_cfg.get('oss') call:
  Replaced with a single _oss_block local variable.

xraysight#7 - get_all() fetches unbounded, client-side limit only:
  Memory.get_all() and Memory.search() both support top_k. Now passed
  server-side so large local vector stores are not over-fetched.
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.

1 participant