Conversation
Signed-off-by: kevin9327 <kevin9327@users.noreply.github.com>
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
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.
Problem
agent.extract(...)drops the system facts the agent was built with.weightsandstrictare forwarded;self._systemis not, so the module-levelextractbuilds itsone-shot agent with
system=None.That matters because system facts are what license relative language. From the docs:
So an agent created the documented way:
extracts without the date it was given, while the module-level call one line away keeps it:
With
strict=True, the default, the loss compounds:_temporal_groundingchecks extracted datesagainst years found in the source, and an unlicensed relative date has nothing to ground against.
Observed on
mainby watching what the inner agent is constructed with:Fix
Forward
self._system, decoded, with an empty string sent asNoneso an agent built without factsbehaves exactly as before.
Tests
Three in
tests/test_weights.py, on the existingenginefixture:test_agent_extract_carries_its_own_system_facts— the facts reachextract. Fails before.test_agent_extract_without_system_facts_sends_none— no facts still meansNone, not"".test_agent_extract_still_carries_weights_and_strict— the two arguments that already worked keepworking, alongside the facts. Fails before.
How I tested
Windows 11, Python 3.12.10.
pytest -q -m "not slow"gives 67 passed, 9 skipped, up from 64 by thethree new tests; nothing else moves. The 9 skips are the six engine tests and the three worker tests
that want a C compiler.
test_lora.py,test_render.py,test_run.py,test_build.pyandtest_finetune.pyare excluded:they need
flax, whoseorbax-checkpointdependency will not install on Windows because its pathsexceed
MAX_PATH. Nothing here touches them, and CI runs them on Ubuntu.One thing worth a maintainer's opinion:
Needle.extractis public but is not inllms.txtordoc/apis.md, which document only the module-levelextract. Happy to add a line for it in eitherfile if you want it documented rather than quietly available.