[#1208] Implemented LinkCreationProcessor::link_creation() - #1222
Open
andre-senna wants to merge 6 commits into
Open
[#1208] Implemented LinkCreationProcessor::link_creation()#1222andre-senna wants to merge 6 commits into
andre-senna wants to merge 6 commits into
Conversation
added 2 commits
August 6, 2026 11:45
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR introduces ChangesLink creation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant LinkCreationProcessor
participant LinkCreationProxy
participant QueryAnswer
participant LinkCreator
LinkCreationProcessor->>LinkCreationProxy: Issue query round
LinkCreationProxy->>QueryAnswer: Drain query answers
LinkCreationProcessor->>LinkCreationProxy: Process query answer
LinkCreationProxy->>LinkCreator: Create links
LinkCreator-->>LinkCreationProxy: Return LinkCreationStats
LinkCreationProcessor->>LinkCreationProxy: Increment round count
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/agents/link_creation_agent/LinkCreationProxy.h (1)
117-117: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the unused
LinkCreatorRegistry::REMOTE_FUNCTIONdependency.
is_link_creation_function_remote()compareslink_creator_function_tagtoLinkCreatorRegistry::REMOTE_FUNCTION, butLinkCreatorRegistry::REMOTE_FUNCTIONis not declared/included fromLinkCreationProxy.h. This currently breaks the build. Since remote evaluation is rejected elsewhere, drop the comparison and use a const local string literal instead of the remote registry tag.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agents/link_creation_agent/LinkCreationProxy.h` at line 117, Update is_link_creation_function_remote() to remove the LinkCreatorRegistry::REMOTE_FUNCTION reference and compare link_creator_function_tag against a const local string literal instead. Do not add the missing dependency or include; preserve the existing remote-evaluation rejection behavior.
🧹 Nitpick comments (3)
src/tests/cpp/link_creation_agent_test.cc (2)
52-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for round handling and processor thread cleanup.
The remote-creator rejection and the
LinkCreationStatsfields are covered. Two behavior changes in this PR are not covered:
LinkCreationProxy::inc_round_count()andstop_criteria_met(): assert thatstop_criteria_met()returns false belowMAX_ROUNDSand true afterMAX_ROUNDScalls toinc_round_count().LinkCreationProcessorthread tracking: assert thatprocessor_threadsreturns to baseline after a command finishes, and that a duplicate thread id raises an error inrun_command.I can draft these cases if you want.
As per coding guidelines: "Test updates are required when production code changes: Add or update C++ tests (*_test.cc under src/tests/cpp/) ... when production code under src/ ... changes behavior".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tests/cpp/link_creation_agent_test.cc` around lines 52 - 56, Extend the existing C++ tests to cover the untested round and processor-thread behavior. Add assertions for LinkCreationProxy::stop_criteria_met() returning false before MAX_ROUNDS and true after MAX_ROUNDS calls to inc_round_count(), and verify LinkCreationProcessor::processor_threads returns to its baseline after a command completes and that run_command rejects a duplicate thread ID.Source: Coding guidelines
42-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCapture each
LinkCreationStatsonce per input.Lines 42-50 call
link_creationnine times to assert three fields for three inputs. Each call builds a newQueryAnswer. Store one stats object per input and assert its three fields. This removes six redundant calls and makes the expected values easier to read.♻️ Proposed refactor
- EXPECT_EQ(proxy1.link_creation(make_shared<QueryAnswer>("blah", 0.0)).created, 4); - EXPECT_EQ(proxy1.link_creation(make_shared<QueryAnswer>("blahhh", 0.5)).created, 6); - EXPECT_EQ(proxy1.link_creation(make_shared<QueryAnswer>("blahh", 1.0)).created, 5); - EXPECT_EQ(proxy1.link_creation(make_shared<QueryAnswer>("blah", 0.0)).updated, 5); - EXPECT_EQ(proxy1.link_creation(make_shared<QueryAnswer>("blahhh", 0.5)).updated, 7); - EXPECT_EQ(proxy1.link_creation(make_shared<QueryAnswer>("blahh", 1.0)).updated, 6); - EXPECT_EQ(proxy1.link_creation(make_shared<QueryAnswer>("blah", 0.0)).visited, true); - EXPECT_EQ(proxy1.link_creation(make_shared<QueryAnswer>("blahhh", 0.5)).visited, true); - EXPECT_EQ(proxy1.link_creation(make_shared<QueryAnswer>("blahh", 1.0)).visited, true); + LinkCreationStats stats1 = proxy1.link_creation(make_shared<QueryAnswer>("blah", 0.0)); + EXPECT_EQ(stats1.created, 4); + EXPECT_EQ(stats1.updated, 5); + EXPECT_TRUE(stats1.visited); + LinkCreationStats stats2 = proxy1.link_creation(make_shared<QueryAnswer>("blahhh", 0.5)); + EXPECT_EQ(stats2.created, 6); + EXPECT_EQ(stats2.updated, 7); + EXPECT_TRUE(stats2.visited); + LinkCreationStats stats3 = proxy1.link_creation(make_shared<QueryAnswer>("blahh", 1.0)); + EXPECT_EQ(stats3.created, 5); + EXPECT_EQ(stats3.updated, 6); + EXPECT_TRUE(stats3.visited);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tests/cpp/link_creation_agent_test.cc` around lines 42 - 50, Update the test around proxy1.link_creation to call it once for each of the three QueryAnswer inputs, storing each returned LinkCreationStats object in a named local variable. Assert created, updated, and visited on the corresponding stored result, preserving all existing expected values.src/agents/link_creation_agent/LinkCreationProcessor.cc (1)
101-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove dead
remove_processor_threadmethod.
remove_processor_threadhas no callers, while the privatethread_process_one_queryself-reap path already erases the finished thread id directly. Delete the method and its declaration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agents/link_creation_agent/LinkCreationProcessor.cc` around lines 101 - 108, Delete the unused LinkCreationProcessor::remove_processor_thread method and its class declaration. Keep the existing thread_process_one_query self-reap logic that erases completed thread IDs directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agents/link_creation_agent/LinkCreationProcessor.cc`:
- Around line 118-160: Prevent the first-round null dereference in the
link_creation loop by guarding or disabling the unfinished
issue_link_creation_query path until it returns a valid proxy. When enabling it,
pass proxy rather than nullptr so command parameters are available, and
consistently drain and test the pattern-matching proxy that owns the
query-answer queue, including the finished/abort checks around pm_proxy.
In `@src/agents/link_creation_agent/LinkCreationProxy.cc`:
- Around line 81-95: Update LinkCreationProxy::link_creation to acquire
api_mutex before reading link_creator_function_tag or
link_creation_function_object, using the existing guarded tag-state access
mechanism. Hold the lock through validation and state retrieval, but release it
before invoking link_creation_function_object->create(answer) so LinkCreator
remains outside the lock.
---
Outside diff comments:
In `@src/agents/link_creation_agent/LinkCreationProxy.h`:
- Line 117: Update is_link_creation_function_remote() to remove the
LinkCreatorRegistry::REMOTE_FUNCTION reference and compare
link_creator_function_tag against a const local string literal instead. Do not
add the missing dependency or include; preserve the existing remote-evaluation
rejection behavior.
---
Nitpick comments:
In `@src/agents/link_creation_agent/LinkCreationProcessor.cc`:
- Around line 101-108: Delete the unused
LinkCreationProcessor::remove_processor_thread method and its class declaration.
Keep the existing thread_process_one_query self-reap logic that erases completed
thread IDs directly.
In `@src/tests/cpp/link_creation_agent_test.cc`:
- Around line 52-56: Extend the existing C++ tests to cover the untested round
and processor-thread behavior. Add assertions for
LinkCreationProxy::stop_criteria_met() returning false before MAX_ROUNDS and
true after MAX_ROUNDS calls to inc_round_count(), and verify
LinkCreationProcessor::processor_threads returns to its baseline after a command
completes and that run_command rejects a duplicate thread ID.
- Around line 42-50: Update the test around proxy1.link_creation to call it once
for each of the three QueryAnswer inputs, storing each returned
LinkCreationStats object in a named local variable. Assert created, updated, and
visited on the corresponding stored result, preserving all existing expected
values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: eeefc3a2-a62d-4758-bb46-bf1852990fcd
📒 Files selected for processing (8)
src/agents/link_creation_agent/LinkCreationProcessor.ccsrc/agents/link_creation_agent/LinkCreationProcessor.hsrc/agents/link_creation_agent/LinkCreationProxy.ccsrc/agents/link_creation_agent/LinkCreationProxy.hsrc/agents/link_creation_agent/link_creators/LinkCreator.ccsrc/agents/link_creation_agent/link_creators/LinkCreator.hsrc/agents/link_creation_agent/link_creators/UnitTestLinkCreator.hsrc/tests/cpp/link_creation_agent_test.cc
💤 Files with no reviewable changes (1)
- src/agents/link_creation_agent/link_creators/LinkCreator.cc
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.
WIP towards #1208
link_creation() is the method where the actual action happens. It uses several parameters to control how and for how long LCA is supposed to keep trying to create links.
I also removed the code related to remote evaluation of link creation requests. This will be returned in a future PR (actually a future issue card). For now, we'll stick with local link creators only.
We are still missing the concrete link creators we use in the evaluation evolution POC. This will be added in a follow-up PR.