fix(skills): extract first JSON object instead of last from multi-object LLM output

[michaelxer]

Summary

_extract_json_object() always paired each { with the last } in the string. When the LLM emitted multiple JSON objects in one response, the first valid object was silently skipped and the later one was persisted as the skill. This caused the extractor to save the wrong skill object when the model produced trailing JSON after the intended result.

Replace the fixed-end scan with brace counting: for each { start, walk forward counting depth until we find the matching }, then try to parse that substring. This returns the first complete JSON object rather than the last.

Target branch

• This PR targets dev, not main. All PRs land in dev; main is curated by the maintainer at each release. If your PR is on main by accident, click "Edit" on this PR and change the base.

Linked Issue

Fixes #3965

Type of Change

• Bug fix (non-breaking — fixes a confirmed issue)

Checklist

• I searched open issues and open PRs — this is not a duplicate.
• This PR targets dev
• My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.

How to Test

1. Run the repro from the issue:

   from services.memory.skill_extractor import _extract_json_object
   r = _extract_json_object('{"title":"First","steps":[]} trailing {"title":"Second","steps":[]}')
   assert r == {"title": "First", "steps": []}  # should be First, not Second

2. Verify the trailing-stray-brace case also works:

   r = _extract_json_object('{"title":"First","steps":[]} }')
   assert r == {"title": "First", "steps": []}

3. Verify normal single-object and code-fenced inputs still work.

[alteixeira20]

Thanks for updating the branch. CI is green now, but this is still not ready to approve.

The PR still only changes services/memory/skill_extractor.py and does not add regression tests. The linked issue asks specifically for an end-to-end maybe_extract_skill() regression where a valid intended skill object is followed by another JSON object, to make sure the wrong skill is not persisted again.

There is also still a parser-safety concern. The current implementation uses brace counting, and the previous audit showed that this rejects valid JSON where braces appear inside string values, for example:

{"title":"Use { brace safely","steps":[]}
{"title":"Use } brace safely","steps":[]}
{"title":"Use \"quoted {\" text","steps":[]}

Please switch to a JSON-aware extraction strategy, ideally using json.JSONDecoder().raw_decode() from each candidate {, or otherwise make the scan string-aware so braces inside quoted strings and escaped quotes do not affect object depth.

Requested coverage:

• _extract_json_object() returns the first complete object when two JSON objects are present;
• _extract_json_object() handles a trailing stray brace after a valid first object;
• _extract_json_object() handles braces inside JSON string values;
• maybe_extract_skill() persists the intended first skill object when another JSON object appears after it.

After that I can re-run the final audit.

[michaelxer]

Pushed the regression tests in a follow-up commit covering all four cases you mentioned: first-of-two-objects, trailing stray brace, braces inside string values, and end-to-end with multi-object output. CI is green.

[michaelxer]

Regression tests covering all four cases are pushed and CI is green. Ready for re-review.

[michaelxer]

Closing this PR as it's been superseded by the merged fix in #3985, which handles ambiguous multi-object outputs at the memory layer. The upstream refactor of also means this branch would need a significant rebase to be relevant. Thanks for the review feedback that guided the better approach.